[performance improvement] Delete outdated hack for legacy JVM #128
Conversation
This change improves performance about 16%. I also tested performance with `s.intern()` but it makes performance 33% slower. note: test environment is OS X 10.11.6, Java(TM) SE Runtime Environment (build 1.8.0_92-b14) Java HotSpot(TM) 64-Bit Server VM (build 25.92-b14, mixed mode)
This optimization can improve performance about 12%. note: test environment is OS X 10.11.6, Java(TM) SE Runtime Environment (build 1.8.0_92-b14) Java HotSpot(TM) 64-Bit Server VM (build 25.92-b14, mixed mode)
|
I wonder how we are 6% slower if we didn't add code, but on the contrary killed quite some... This PR covers the contents of findbugsproject/findbugs#132 regarding string canonicalization (original implementation), I strongly support it. The rest is an alternative of part (not all) the changes suggested on findbugsproject/findbugs#135, you may want to look at the other changes proposed in that PR. findbugsproject/findbugs#134 also introduces performance improvements (avoid an array allocation and iniialization). |
|
|
||
| } | ||
| /* | ||
| * Andrei, 27.02.2008: "optimized" code below takes ~18% overall FB |
There was a problem hiding this comment.
this comment should be removed if the implementation is dropped.
I've run the same benchmark with Apache Cassandra 3.1.0, which I believe that really big project (6.3 MB, about 3,600 classes). This optimisation could improve performance from 284s to 265s. I think we can merge this fix if we have no another OSS which can prove potential problem. I will try to introduce this change to this branch. |
|
findbugsproject/findbugs/pull/134 doesn't affect performance, but it's more intuitive implementation so I will add it to this pull-request. |
There is no real need to copy things over and over
I tried introducing Table in a9ac183, but it cannot improve performance (25.83s -> 26.46s) in microbench with Guava 19.0. |
|
@KengoTODA I'm theorizing here, but the main key for the table is the method name, under high overloading this means several conflicts... Also, 2 nested maps + an array means lots of jumps in memory to get to the value... Possibly simplifying the table to: class MethodDescriptor {
private final String name;
private final String signature;
private final boolean static;
// hashCode() and equals()
}
Map<MethodDescrptor, XMethod> table = new HashMap<>();may produce better results. But there is only one way to know for sure... |
These methods have been deprecated and turned into no-ops with spotbugs#128. This change removes the calls (but not the methods themselves, so not to break API) to reduce the number of distracting warnings.
These methods have been deprecated and turned into no-ops with #128. This change removes the calls (but not the methods themselves, so not to break API) to reduce the number of distracting warnings.
|
note: I run a microbenchmark on cloud (could be unstable), and it says that https://github.com/KengoTODA/spotbugs-benchmark/runs/1362695598?check_suite_focus=true#step:7:36 |
According to my benchmark, SpotBugs is 6% slower than FindBugs 3.0.1. And some of bottlenecks can be solved easily without adding new dependency nor upgrading library. This pull-request proposes these easy fixes.
Benchmark script
I use Google Guava as target, because it has no big dependencies but it depends on JSR305 so we can use almost full-set feature of SpotBugs.
Note
timemight not be the same one if you're using different OS.HashMap#get(key)costs much time even thoughMethodDescriptorimplementshashCode()andequals()in acceptable quality.HashMap#get(key)costs much time even thoughClassDescriptorimplementshashCode()andequals()in acceptable quality.HashMap#get(key)costs much time even though we useStringas key.O(n)algorithm to search. Better to introduce Table or some other collection to search byO(log(n))algorithm.