Bug 365966

Comment 1 William Swanson 2011-12-07 17:00:05 EST

Just confirming that I wrote the original subclasses of RequestMonitor
in the Visualizer code. (Although to be fair, as written these were
thin convenience wrappers around the base classes. The chief benefit
here is in simplifying code that uses immediate monitors.)

Comment 2 Marc Khouzam 2011-12-08 09:16:33 EST

(In reply to comment #1)
> Just confirming that I wrote the original subclasses of RequestMonitor
> in the Visualizer code. (Although to be fair, as written these were
> thin convenience wrappers around the base classes. The chief benefit
> here is in simplifying code that uses immediate monitors.)

Thanks Bill.

Can you attach the patch with the three classe with the following Copyright header (put your name in the form you prefer).

Can you also state that you have the right to contribute this code.

Since this patch is < 250 lines, once the two points above are done, I will be able to commit, assuming Pawel is ok with it.


/*******************************************************************************
 * Copyright (c) 2011 Tilera and others.
 * All rights reserved. This program and the accompanying materials
 * are made available under the terms of the Eclipse Public License v1.0
 * which accompanies this distribution, and is available at
 * http://www.eclipse.org/legal/epl-v10.html
 * 
 * Contributors:
 *     William Swanson (Tilera) - initial API and implementation (Bug 365966)
 *******************************************************************************/

Comment 3 Pawel Piech 2011-12-08 10:24:45 EST

(In reply to comment #2)
> Since this patch is < 250 lines, once the two points above are done, I will be
> able to commit, assuming Pawel is ok with it.
Makes sense to me :-)  Thank you for the contribution.

Comment 4 Marc Khouzam 2011-12-08 10:46:34 EST

(In reply to comment #3)
> Makes sense to me :-)  Thank you for the contribution.

Thanks Pawel.

I went through dsf.gdb, dsf.gdb.ui and tests.dsf.gdb and made use of the new convenience Immediate*RequestMonitor classses everywhere the ImmediateExecutor was being used.  I then ran the JUnit tests and got not new errors.

I will wait for Bill to post the patch with the proper copyright and commit all these changes.

Comment 5 Marc Khouzam 2011-12-08 10:47:52 EST

Created attachment 208099 [details]
Changes to DSF-GDB to use new classes

This patch changes all of DSF-GDB to use the new classes.  It was not necessary but it was my way of testing the new classes.

Comment 6 Marc Khouzam 2011-12-08 14:00:32 EST

I'm noticing a side-effect of using ImmediateRMs a lot.
If the job of the RM is large, like updating a thousand threads in the Visualizer, using the ImmediateExecutor keeps a hold on the DSF executor until the entire work is done.

Because we sometimes use a Query in the UI thread, which requires the use of the DSF executor, the UI locks up waiting for the DSF Executor to become free.

The real problem is the locking of the UI because of our use of Query.  We may be able to fix that.  I know Mikhail is having the same problem in Bug 365601.
But it is something to be aware of, as we may choose to not always use the ImmediateExecutor as a trick to allow the DSF executor to become available more often.

Comment 7 William Swanson 2011-12-09 16:52:52 EST

Created attachment 208208 [details]
Patch file: Immediate monitor classes

I've attached the relevant git patch file for this bug.

Comment 8 Marc Khouzam 2011-12-12 05:39:16 EST

(In reply to comment #7)
> Created attachment 208208 [details]
> Patch file: Immediate monitor classes
> 
> I've attached the relevant git patch file for this bug.

Thanks Bill.

Can you state in this bug that:

1) you are entitled to submit this code
2) you wrote the code yourself

Comment 9 William Swanson 2011-12-12 10:21:44 EST

(In reply to comment #8)

Just to confirm:
1) I have approval to submit this code.
2) I wrote this code myself.

Comment 10 Marc Khouzam 2011-12-12 11:30:55 EST

Pushed to master.

Thanks Bill!

Comment 11 Marc Khouzam 2011-12-12 11:31:26 EST

I also pushed the changes to DSF-GDB to use the new classes.

Comment 12 CDT Genie 2011-12-12 12:23:03 EST

*** cdt git genie on behalf of Marc Khouzam ***

    Bug 365966: Change all of DSF-GDB to use the new Immediate*RequestMonitor classes.  Not a necessary change, but it makes the code easier to read, and validates the new classes.

[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=fb16cb05614cfe6ac1776a8780641b0e5dc505cd

Comment 13 CDT Genie 2011-12-12 12:23:05 EST

*** cdt git genie on behalf of William R. Swanson ***

    Bug 365966: Convenience RequestMonitor classes using ImmediateExecutor

[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=1291c9844716ada5ac164fbb0527153ec0098530