Community
Participate
Working Groups
I have a properties with a hierarchy. Everything is read-only except for the leaf nodes. Setting a value takes about 5 seconds on my Core2Duo and seems to have possibly gotten even slower in RC2. This stems from the call to PropertySheetEntry.refreshFromRoot in the setValue method. Why should 1 value cause a total refresh? If other values were dependent, shouldn't they have listeners? This is a big problem for me as it will be very hard for my users to be productive with such a slow interface. I realize its late to get something in for 3.3. If there is a workaround, I would really like to know about it so I don't have to reimplement the property view.
Adam, just to get a ballpark on how bad this is how many preperties are there in the tree in total? I'm marking this as 3.3.1 for now...
Thanks for looking at this Eric. I have about 300 editable properties and about 40 non-editable sources that provide the hierarchy. Btw - I did some testing with changing the call to refreshFromRoot() in PropertySheetEntry.setValue to refreshValues(). This seemed to fix this problem for me without breaking anything. However, I would in no way call this testing extensive. Do you know why refreshFromRoot has to be used here? (In reply to comment #1) > Adam, just to get a ballpark on how bad this is how many preperties are there > in the tree in total? > > I'm marking this as 3.3.1 for now... >
Adam, let me take a look (at least to see if there have been changes here). It seems to be at least 'suspicious' that changing an individual property should have to globally refresh. That being said it's also suspicious that this call is taking 5 secs (I'd expect that we should able to do this with only 300 entries in 'real time' like the new QuickAccess (Ctrl-3) does when it filters its tree as you type).
Created attachment 69535 [details] Patch that demonstrates the issue This patches org.eclipse.ui.examples.propertysheet by changing the 'Address' to create 'n' copies of its properties. It also patches the 'setValue' call to instrument how long the 'refreshFromRoot' took and how many properties eventually got a chance to 'update'.
Adam, The above patch demonstrates your issue I think...I tested by changing the value in the for loop in the Address class and restarting an inner, opening a '.usr' file and selecting a name (with the Properties view open). Then I -expanded- the address field before changing the 'Name' att and hitting enter. my initial results for various 'n' values is: Count Time(ms) Updates 1 15 14 100 2922 410 200 11382 810 300 24781 1210 400 44859 1610 500 68968 2010 Haven't graphed it yet but it's definitely non-linear...
wow - thanks, Eric. this looks consistent with what I'm seeing. Any chance simply replacing the refreshFromRoot would work as I suggested? I'm guessing its there for a reason and its going to require a more involved fix. (In reply to comment #5) > Adam, The above patch demonstrates your issue I think...I tested by changing > the value in the for loop in the Address class and restarting an inner, opening > a '.usr' file and selecting a name (with the Properties view open). Then I > -expanded- the address field before changing the 'Name' att and hitting enter. > > my initial results for various 'n' values is: > > Count Time(ms) Updates > > 1 15 14 > 100 2922 410 > 200 11382 810 > 300 24781 1210 > 400 44859 1610 > 500 68968 2010 > > Haven't graphed it yet but it's definitely non-linear... >
Yeah, problem with changes like that is that it's likely to break someone building on top of us (RCP...) that relies on the current behavior. My guess is that the 'refreshFromRoot' needs some (un)optimization (my unsubstantiated suspicion is that we're trying to minimize the 'churn' but in the process actually doing too much merging...
Created attachment 76785 [details] Patch to use a HashMap to find TreeItem's corresponding to a particular PropertySheet entry
Here's the new times with the fix in place: Count Time(ms) Updates 1 0 14 100 125 410 200 234 810 300 312 1210 400 453 1610 500 531 2010 much more linear...;-)
Committed to HEAD in >20070824. Now uses a map to find the TreeItems associated with an entry.
Committed to R3_3_maintenance in >20070824. Will verify in both after the next M-build.
*** Bug 197310 has been marked as a duplicate of this bug. ***
Verified in I20070904-0800. Still need to verify the maintenance fix. Verified by applying the patch to the example -only-, running an inner and chaning a name.
Verified in M20070905-1045.
