-
Notifications
You must be signed in to change notification settings - Fork 327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optimize PackageInfoService #1136
Conversation
Can drastically reduce icon encoding times.
Don't fetch all the packages to just search for a single package. Shaves off half a second or so.
Don't check again if you already just checked. Shaves off a second or so.
Reduce worst-case latency.
78bbcec
to
d492248
Compare
Collections.sort(sorted, new Comparator<Counter>() { | ||
@Override | ||
public int compare(Counter a, Counter b) { | ||
if (a.time < b.time) { return 1; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return Float.compare(b.time, a.time);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
private final Counter counter; | ||
|
||
private Scope(Counter counter) { | ||
this.start = System.currentTimeMillis(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You probably should use System.nanoTime
, since System.currentTimeMillis
may not be monotonic and has very loose guarantees.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
} else { | ||
return mMap.get(pngBase64); | ||
} | ||
} finally { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are undoing most of this change in the next commit. Maybe squash the 'add try, remove try' parts?
|
||
/** | ||
* Lookups the value from the cache using the given key. If the cache does not contain the value | ||
* then the {@link Builder} will be used to create the value. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment that this is not thread safe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made thread safe instead.
private FileCache() { | ||
} | ||
|
||
static <K, V> Cache<K, V> create( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this not public?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
} | ||
} | ||
|
||
public V scopedBuild(K key) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be private.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
for (int n; (n = is.read(buf)) != -1; ) { | ||
os.write(buf, 0, n); | ||
} | ||
os.flush(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove. It does nothing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Reduces package info with icons from ~17 seconds down to 2.5s (warm cache), 8s (cold cache) on my test device.
Fixes: #1000