eclipse-platform / eclipse.platform

https://eclipse.dev/eclipse/
Eclipse Public License 2.0
84 stars 113 forks source link

ElementTree#includes: relax synchronized access #1531

Closed jukzi closed 2 months ago

jukzi commented 2 months ago

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.

jukzi commented 2 months ago

Without this change Flighrecorder shows 4 sec locks during filesearch in platform workspace: image

With this change this reduces to 2 sec: image

However the perfromance benefit its hardly noticable by the user (only a few milli seconds less)

github-actions[bot] commented 2 months ago

Test Results

 1 734 files  ±0   1 734 suites  ±0   1h 30m 34s :stopwatch: + 6m 24s  3 979 tests ±0   3 956 :white_check_mark: ±0   22 :zzz: ±0  1 :x: ±0  12 534 runs  ±0  12 367 :white_check_mark: ±0  164 :zzz: ±0  3 :x: ±0 

For more details on these failures, see this check.

Results for commit 3a0c9c5f. ± Comparison against base commit 2fb8958c.

:recycle: This comment has been updated with latest results.

jukzi commented 2 months ago

ignoring unrelated failing test "TocLinkChecker.testPdeUserGenerated" - already failed in IBuild