Community
Participate
Working Groups
Created attachment 188215 [details] Patch v.0.1 In the fix for bug 335591 we have added additional initialization in #load. This fixed the problem however resulted in subsequent child node creation and unnecessary preferences loading. We should avoid unnecessary node creation because of performance reasons. The same applies to subsequent loads because #load reads data from disk which can be quite expensive. What we could do is to postpone loading until the project node is initialized. To get correct preference value, the following preconditions would have to be met: 1. Project is open. 2. Project node is initialized. This can be done if point 1 is satisfied by creating project node or by calling directly #initialize on already created project node. 3. Load level node is loaded. This can be done if point 2 is satisfied by calling #load during node creation, by calling #read when prefs file content has changed or by trying to access some preference stored in this node for the first time (#internalGet). Attached patch shows the initial approach.
Created attachment 188448 [details] Patch v.0.2
DJ, what do you think about the above enhancement?
I'm not sure I quite understand the use case and what is happening. Can you explain it in a bit more detail? I thought that the old code only loaded nodes at the load level, but from this bug description I guess that isn't the case anymore? The attached test case has an example of setting project-level preference values for closed projects. Is this a scenario that we want to even support?
(In reply to comment #3) > I'm not sure I quite understand the use case and what is happening. I think the patch can be misleading because it mixes at least two different ideas: lazy loading and removing unnecessary node creation if the node is already created. I should split it into separate bugs to avoid confusion. > Can you > explain it in a bit more detail? I thought that the old code only loaded nodes > at the load level, but from this bug description I guess that isn't the case > anymore? Loading is still done at the load level but in the patch it is postponed until the first use (set or get) of this node occurs. After second thought, I am not sure that this is a big improvement and is really needed. Preference files are not that big and usually when we create a node we try to set or get a value immediately. > The attached test case has an example of setting project-level preference > values for closed projects. Is this a scenario that we want to even support? This is allowed to set project-level preference values for closed projects without storing the node on disk. It is not a good pattern but it is possible.
Szymon P., as you said the patch tries to address multiple issues. Let's split the bug into smaller ones and discuss them one by one. Could you tell what the performance gain is when the lazy loading is applied?
*** Bug 415750 has been marked as a duplicate of this bug. ***
As mentioned in comment 4, the original patch handled two issues: lazy loading and removing unnecessary node creation if the node was already created. I will use this bug to fix the removing unnecessary node creation part. Lazy-loading is not backed by any performance measures so I will drop this idea for now until we get some better rationale to implement it.
Fixed in master: http://git.eclipse.org/c/platform/eclipse.platform.resources.git/commit/?id=f71edcf4c3eb45810bee0ab883e34327fac33499 Tobias, since you found a real problem in your product because of this bug, I'd appreciate if you could verify if the fix solves your problem. Thanks in advance!
In my setup it works fine now :)
