Hi Gary and BCEL maintainers, I've added Javadoc for the enhancement. https://github.com/apache/commons-bcel/pull/26 I appreciate if you can check the direction of the implementation is good or bad.
Regards, Tomo On Sun, May 19, 2019 at 11:40 PM Tomo Suzuki <suzt...@google.com> wrote: > Hi Gary (and BCEL maintainers), > > Thank you for the comment. It has been addressed. Would you check the pull > request? > https://github.com/apache/commons-bcel/pull/26/files > > Regards, > Tomo > > On Wed, May 8, 2019 at 17:46 Tomo Suzuki <suzt...@google.com> wrote: > >> Hi Gary, >> Created a draft PR to receive feedback. >> https://github.com/apache/commons-bcel/pull/26/files . What do you >> think? >> >> Regards, >> Tomo >> >> From: Gary Gregory <garydgreg...@gmail.com> >> Date: Wed, May 8, 2019 at 9:40 AM >> To: Commons Developers List >> >> > Great. I look forward to seeing what you'll come up with :-) >> > >> > On Tue, May 7, 2019 at 4:27 PM Tomo Suzuki <suzt...@google.com.invalid> >> > wrote: >> > >> > > I found the discussion on getInstance method had incurred performance >> > > degradation https://issues.apache.org/jira/browse/BCEL-186 . >> > > >> > > From the JIRA: >> > > > This feature could return as a pluggable cache if someone wants to >> > > provide a patch >> > > >> > > I'll think about the approach. >> > > >> > > Regards, >> > > Tomo >> > > >> > > >> > > On Thu, May 2, 2019 at 1:22 PM Gary Gregory <garydgreg...@gmail.com> >> > > wrote: >> > > >> > > > On Thu, May 2, 2019 at 10:59 AM Tomo Suzuki >> <suzt...@google.com.invalid> >> > > > wrote: >> > > > >> > > > > Gary, >> > > > > I didn't find ConstantUtf8.getCachedInstance >> > > > > < >> > > > >> > > >> https://github.com/apache/commons-bcel/blob/master/src/main/java/org/apache/bcel/classfile/ConstantUtf8.java#L94 >> > > > > >> > > > > is used in BCEL (latest 6.3.1). I tried my custom ConstantUtf8 >> > > > modification >> > > > > to leverage getCachedInstance method and it worked well: >> > > > > >> > > > > Before >> > > > > >> > > > > Without getCachedInstance, my tool created 2,6 million >> ConstantUtf8 >> > > > > instances (before failing OutOfMemoryError: GC overhead limit >> exceeded) >> > > > to >> > > > > check ~200 jar files >> > > > > [image: 2644k_constantutf8_without_getCachedInstance.png] >> > > > > >> > > > > >> > > > > After >> > > > > >> > > > > With getCachedInstance, my tool created just 0.6 million >> ConstantUtf8 >> > > > > instances. It didn't throw the OutOfMemoryError. >> > > > > [image: 621k_constantutf8_with_getCachedInstance.png] >> > > > > >> > > > > My modification was to use getCachedInstance for >> > > > > ConstantUtf8.getInstance(DataInput) >> > > > > < >> > > > >> > > >> https://github.com/apache/commons-bcel/blob/master/src/main/java/org/apache/bcel/classfile/ConstantUtf8.java#L123 >> > > > > >> > > > > . >> > > > > >> > > > > >> > > > > Do you know why ConstantUtf8.getCachedInstance is unused? >> > > > > >> > > > >> > > > I do not. >> > > > >> > > > Gary >> > > > >> > > > >> > > > > >> > > > > Regards, >> > > > > Tomo >> > > > > >> > > > > >> > > > > >> > > > > On Wed, May 1, 2019 at 7:02 PM Gary Gregory < >> garydgreg...@gmail.com> >> > > > > wrote: >> > > > > >> > > > >> On Wed, May 1, 2019 at 6:37 PM Tomo Suzuki >> <suzt...@google.com.invalid >> > > > >> > > > >> wrote: >> > > > >> >> > > > >> > Gary, >> > > > >> > That’s right. I missed it. I think I just need to find a way to >> > > > leverage >> > > > >> > the getCachedInstance method. Thank you for quick response. >> > > > >> > >> > > > >> >> > > > >> YW. Please let us know your outcome. >> > > > >> >> > > > >> Gary >> > > > >> >> > > > >> >> > > > >> > >> > > > >> > On Wed, May 1, 2019 at 18:00 Gary Gregory < >> garydgreg...@gmail.com> >> > > > >> wrote: >> > > > >> > >> > > > >> > > Why not use getCachedInstance() ? >> > > > >> > > >> > > > >> > > Gary >> > > > >> > > >> > > > >> > > On Wed, May 1, 2019 at 5:20 PM Tomo Suzuki >> > > > <suzt...@google.com.invalid >> > > > >> > >> > > > >> > > wrote: >> > > > >> > > >> > > > >> > > > Hi BCEL developers, >> > > > >> > > > >> > > > >> > > > We use BCEL library to inspect Java class. Thank you for >> the >> > > great >> > > > >> > > library. >> > > > >> > > > >> > > > >> > > > When our tool checks classes in ~200 jar files, it creates >> more >> > > > >> than 2 >> > > > >> > > > million BCEL ConstantUtf8 instances. I suspect many of them >> > > share >> > > > >> the >> > > > >> > > same >> > > > >> > > > values such as "java.lang.String". >> > > > >> > > > >> > > > >> > > > [image: many_constant_utf8.png] >> > > > >> > > > >> > > > >> > > > Given this observation, I got an idea to to reuse >> ConstantUtf8 >> > > > with >> > > > >> > same >> > > > >> > > > value to save memory footprint. Because ConstantUtf8 is >> > > constant, >> > > > >> > sharing >> > > > >> > > > the instances across classes should not cause a problem. >> Note >> > > that >> > > > >> BCEL >> > > > >> > > is >> > > > >> > > > not designed thread-safe (doc >> > > > >> > > > <https://commons.apache.org/proper/commons-bcel/faq.html >> >). >> > > > >> > > > >> > > > >> > > > If this is a good idea, I'm thinking to make a pull request >> > > around >> > > > >> > BCEL's >> > > > >> > > > ConstantUtf8.getInstance method: >> > > > >> > > > >> > > > >> > > > >> > > > >> > > >> > > > >> > >> > > > >> >> > > > >> > > >> https://github.com/apache/commons-bcel/blob/master/src/main/java/org/apache/bcel/classfile/ConstantUtf8.java#L116 >> > > > >> > > > >> > > > >> > > > Does anybody have any thought? >> > > > >> > > > >> > > > >> > > > -- >> > > > >> > > > Regards, >> > > > >> > > > Tomo >> > > > >> > > > >> > > > >> > > >> > > > >> > -- >> > > > >> > Regards, >> > > > >> > Tomo >> > > > >> > >> > > > >> >> > > > > >> > > > > >> > > > > -- >> > > > > Regards, >> > > > > Tomo >> > > > > >> > > > >> > > >> > > >> > > -- >> > > Regards, >> > > Tomo >> > > >> >> >> >> -- >> Regards, >> Tomo >> > -- > Regards, > Tomo > -- Regards, Tomo
