Bug 281327 - Patch for DeferredUpdateManager: Replace RunnableChain by List
Summary: Patch for DeferredUpdateManager: Replace RunnableChain by List
Status: NEW
Alias: None
Product: GEF
Classification: Tools
Component: GEF-Legacy Draw2d (show other bugs)
Version: 3.4   Edit
Hardware: PC All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Anthony Hunter CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-06-24 05:26 EDT by Micha Riser CLA
Modified: 2013-10-17 09:44 EDT (History)
2 users (show)

See Also:


Attachments
A patch that queues Runnable in an ArrayList instead of creating a RunnableChain (3.41 KB, patch)
2009-06-24 05:26 EDT, Micha Riser CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Micha Riser CLA 2009-06-24 05:26:22 EDT
Created attachment 139957 [details]
A patch that queues Runnable in an ArrayList instead of creating a RunnableChain

Build ID: M20080911-1700

Steps To Reproduce:
In the class org.eclipse.draw2d.DeferredUpdateManager, Runnables are queued using a special inner class RunnableChain which basically produces a linked list of Runnables. When run is called on the head of the linked list, first run of its next element is called an then run on the runnable of the elment itself.

This produces very large stacks, for each element in the list, a new stack frame is created. Instead of the RunnableChain construct, a simple ArrayList can be used as queue which will be faster and use less memory.
Comment 1 Anthony Hunter CLA 2010-02-04 19:22:00 EST
This issue was also raised in Bug 137409 .
Comment 2 Anthony Hunter CLA 2010-03-12 14:44:18 EST
Not API so can move to M7.

This has been identified as a performance improvement so we need to review the patch.
Comment 3 Alex Boyko CLA 2010-03-14 18:08:05 EDT
Overall, the patch looks good tome. One thing, though. Why did you move declaration of local variables inside the loop instad of leaving them out as they used to be? (Methods: #pervformValidation() and repairDamage()). I'd rather leave that part as it used to be.
Comment 4 Micha Riser CLA 2010-03-15 05:46:12 EDT
The reason I move the variable declarations was to make it more readable to me to understand what the original code was doing.. Declaring the variables in the innermost block that they are used makes their scope more clear. This seems to be in compliance with the coding convention http://java.sun.com/docs/codeconv/html/CodeConventions.doc5.html#2991 point 6.3. It is just a detail, however.
Comment 5 Anthony Hunter CLA 2010-05-03 13:10:20 EDT
We released 3.6 M7 today so moving unresolved bugs to 3.6 RC1.

We need to re-access if we can complete these for Helios.
Comment 6 Anthony Hunter CLA 2010-05-17 10:42:11 EDT
Unfortunately this is not going to make Helios, we ran out of testing time.
Comment 7 Micha Riser CLA 2011-01-17 08:42:27 EST
Bump. What about including it in the next eclipse release?
Comment 8 Alexander Nyßen CLA 2013-10-17 09:44:36 EDT
Unset target milestone as the specified one is already passed.