Skip to content

Commit c0973ff

Browse files
EcljpseB0Tjukzi
authored andcommitted
ElementTree#includes: relax synchronized access
lookupCache is declared volatile and it's type DataTreeLookup has all fields final, so they can be used outside the synchronized block. Sadly same pattern does not work for getElementData() as the returned Object is not guaranteed to be thread-safe but some checks can be moved outside the synchronized block. ElementTree.includes() is heavily used for example during file search.
1 parent 2fb8958 commit c0973ff

File tree

1 file changed

+16
-9
lines changed
  • resources/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/watson

1 file changed

+16
-9
lines changed

resources/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/watson/ElementTree.java

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -385,15 +385,20 @@ public DeltaDataTree getDataTree() {
385385
* Returns the element data for the given element identifier.
386386
* The given element must be present in this tree.
387387
*/
388-
public synchronized Object getElementData(IPath key) {
388+
public Object getElementData(IPath key) {
389389
/* don't allow modification of the implicit root */
390-
if (key.isRoot())
390+
if (key.isRoot()) {
391391
return null;
392-
DataTreeLookup lookup = lookupCache; // Grab it in case it's replaced concurrently.
393-
if (lookup == null || lookup.key != key)
394-
lookupCache = lookup = tree.lookup(key);
395-
if (lookup.isPresent)
396-
return lookup.data;
392+
}
393+
synchronized (this) {
394+
DataTreeLookup lookup = lookupCache; // Grab it in case it's replaced concurrently.
395+
if (lookup == null || lookup.key != key) {
396+
lookupCache = lookup = tree.lookup(key);
397+
}
398+
if (lookup.isPresent) {
399+
return lookup.data;
400+
}
401+
}
397402
throw createElementNotFoundException(key);
398403
}
399404

@@ -545,10 +550,12 @@ public synchronized void immutable() {
545550
* Returns true if this element tree includes an element with the given
546551
* key, false otherwise.
547552
*/
548-
public synchronized boolean includes(IPath key) {
553+
public boolean includes(IPath key) {
549554
DataTreeLookup lookup = lookupCache; // Grab it in case it's replaced concurrently.
550555
if (lookup == null || lookup.key != key) {
551-
lookupCache = lookup = tree.lookup(key);
556+
synchronized (this) {
557+
lookupCache = lookup = tree.lookup(key);
558+
}
552559
}
553560
return lookup.isPresent;
554561
}

0 commit comments

Comments
 (0)