Community
Participate
Working Groups
Created attachment 119963 [details] PoC patch Build ID: 3.4.1.M20080827-0800a Steps To Reproduce: I am a performance analyst for an adopting product. During analysis of our forthcoming release (based on Eclipse 3.4.1), we observed a lot of stress on the locks used in the eclipse.jobs plugin. One source of this stress is coming from the org.eclipse.ui.internal.decorators.DecorationScheduler class, which is calling wakeUp() and schedule() on its 'decorationJob' thousands of times during a build of a medium-sized workspace. Since this is UI-specific, there should be a check for when the last time the wakeUp() request was made, and only make the request if enough time has elapsed (such as 100 ms). This will ease the stress on the jobs locks slightly. This follows a general pattern that we are seeing, that updates to the UI are made much too quickly, using a lot of system resources. In UI-specific plugins, care should always be taken to not do things too often, thus avoiding unnecessary processing. I have attached a proof-of-concept patch (on R3_4_maintenance) that reduces the number of times these methods are called dramatically for my workload (e.g. from > 17k times to < 1k times).
Randall, how much effect does your patch have on the elapsed time of the build?
I have measured an average improvement of about 1 sec (slightly over 1%) for subsequent builds. However, I failed to mention that I ran into this when investigating high variability in the amount of time getting Java locks. I opened this bug to mostly help with the variability (by reducing some of the strain on the hot locks).
I don't actually see why wakeUp is called at all here. There are two ways a job can get into the sleeping state: 1) Job.sleep() is called. In this case there must be a future call to wakeUp() or it will sleep forever 2) Job.schedule() is called, and the job scheduler decides to let the job sleep for awhile (it does this for jobs with DECORATE priority when the system is under load) In DecorationScheduler, 1) never happens as far as I can see, which means the job is sleeping because schedule() has been called. In this case wakeUp() will just remove the job from the sleep queue, and then re-insert it, possibly in a slightly different position. I can see his has overhead because it's removing an item from a linked list and then re-inserting it, which are both O(N) operations. I would suggest removing the wakeUp() call entirely and just leave the schedule() call there. schedule() is relatively quick when a job is already in the sleep or wait queues. I would suggest trying this out in 3.5 for awhile before back-porting to a maintenance release though. A more conservative optimization is to avoid calling schedule() in the case wakeUp() has been called. Calling schedule() right after wakeUp() is guaranteed to have no effect. I.e., change this: if (decorationJob.getState() == Job.SLEEPING) { decorationJob.wakeUp(); } decorationJob.schedule(); To this: if (decorationJob.getState() == Job.SLEEPING) { decorationJob.wakeUp(); } else { decorationJob.schedule(); }
Hi Kevin, I just set the milestone to M5 thinking the bug was assigned to me. Sorry! Feel free to undo that or change to a different target.
(In reply to comment #3) > I don't actually see why wakeUp is called at all here. > I would suggest removing the wakeUp() call entirely and just leave the > schedule() call there. schedule() is relatively quick when a job is already in > the sleep or wait queues. Thanks John. I've tried as you suggested and it seems to perform just fine, and still quite perky - e.g. expanding trees of projects brought in from CVS, the CVS info appears just as fast (subjectively), even with builds running. I've noticed some pauses but also with the old code, plus we have the UI job in the mix to do the actual painting, so not sure this change is a factor. I assume the code was as it was because of concern that the job needed to be "kicked" when new work was needed to be performed. But as you indicated, schedule() along is enough.
Released to HEAD
Verified in 0428-0100 that the code is released and that decorators still work in a timely fashion.
Comment on attachment 119963 [details] PoC patch Removing iplog+ flag, this patch wasn't released.
