Repository navigation
DO NOT MERGE, brute force change in LocationMap - #2253
iloveeclipse wants to merge 1 commit into
Conversation
LocationMap should use HashMap for better performance Fixes eclipse-platform#2251
| matching = map; | ||
| } | ||
| Map<IFileStore, Object> matching = map; | ||
| for (Object value : matching.values()) { |
There was a problem hiding this comment.
Don't we need to iterate over the entryset here and check that key is actually greater than endpoint if prefixParent != null?
Before we have only passed a subset to the consumer now its always the whole map what makes it feel wrong given the name is "matchingPrefixDo"
There was a problem hiding this comment.
Please don't review draft PR's with the prefix "DON'T MERGE".
This one is simply to see if there are any tests available that validate exactly this removed code.
See #2252 (comment)
| * Map of FileStore->IResource OR FileStore->ArrayList of (IResource) | ||
| */ | ||
| private final SortedMap<IFileStore, Object> map = new TreeMap<>(IFileStore::compareTo); | ||
| private final Map<IFileStore, Object> map = new HashMap<>(10_000); |
There was a problem hiding this comment.
Form the javadoc
Iteration over collection views requires time proportional to the "capacity" of the HashMap instance (the number of buckets) plus its size (the number of key-value mappings). Thus, it's very important not to set the initial capacity too high (or the load factor too low) if iteration performance is important.
Maybe we should start smaller (or use a system property to control the inital size)
Test Results 1 866 files - 81 1 866 suites - 81 1h 24m 45s ⏱️ -57s For more details on these failures and errors, see this check. Results for commit d1e15a2. ± Comparison against base commit f752dd4. This pull request removes 153 tests. |
|
Good to know there are tests for this code and they also fail if something is wrong :-) |
2 participants
LocationMap should use HashMap for better performance
Fixes #2251