Hi Konstantin, Thank you very much for the feedback.
> I have the following concerns: > > 1) Someone was testing Tomcat usage of BCEL and found that using this > caching did not improve performance, but reduced it for our use case. > It was reported in the following Bugzilla issue: > > https://issues.apache.org/bugzilla/show_bug.cgi?id=56940 Do you have more details on the size of the jars used to measure the performance? Did the test let enough time for the JIT to kick in? > 2) Configuration of this cache depends on reading System properties such as > > final static boolean BCEL_DONT_CACHE = Boolean.getBoolean("bcel.dontCache"); > > It is a bit odd to me to configure a library via system properties. > > At least there could be a static setter for that flag, or a static > setter for a cache instance / a factory class. I agree, a system property was probably good for Findbugs if it forks its own VM, but not for a general purpose library. > 3) RELEASE-NOTES.txt describes this change as > > [BCEL-163] Incorporate patch file from Findbugs > > which does not say much about this change. > > Actually this change introduced caching for ConstantUtf8 and > ObjectType instances. I'll update it if I roll a new RC. Emmanuel Bourg --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
