proposal for #2209 #2216
proposal for #2209 #2216
Conversation
|
@dfa1 can you post the before and after numbers for your local environment? thanks |
|
Before (master 6ffdd2d): This branch: Better throughput but also bigger avg time! NB: @andimarek since this benchmark is using |
|
interesting! Without master: this branch: |
| @@ -77,7 +74,7 @@ private MergedField(List<Field> fields) { | |||
| * @return the name of of the merged fields. | |||
| */ | |||
| public String getName() { | |||
| return singleField.getName(); | |||
| return fields.get(0).getName(); | |||
There was a problem hiding this comment.
Rather than do a list by index look up everytime we need name, do it once at construction into a String name field
There was a problem hiding this comment.
Or cache the field since its used everywhere
Benchmark Mode Cnt Score Error Units BenchMark.benchMarkSimpleQueriesThroughput thrpt 15 55.953 ± 1.093 ops/s BenchMark.benchMarkSimpleQueriesAvgTime avgt 15 18.106 ± 0.245 ms/op
|
Tested the idea quickly, since the Benchmark is always using one field per Before (master 6ffdd2d): So this is interesting: there is some overhead that we could kill between |
|
@bbakerman @andimarek I give up on this PR.... I hope that we have some more data about what is the bottleneck! |
|
Sorry @dfa1 that we didn't come back in time but as you know it happen sometimes. |
|
no worries @andimarek! I tried several things and none of them worked 100% for all use cases! |
@andimarek tentative implementation for avoiding some copies in MergedField&co
On my old macbook (2013) I get these numbers:
usually I get 50 ops/s in this benchmark . Can you please double check?
/cc @bbakerman not sure if I'm following the "mutable builder"/"immutable object" pattern. For sure, the type signature are much hard to read now.... please advise :)