Hi Gary and BCEL maintainers, I created a PR for LruCacheClassPathRepository https://github.com/apache/commons-bcel/pull/28 . I appreciate if you can review them.
Regards, Tomo On Mon, Jun 17, 2019 at 12:11 PM Tomo Suzuki <suzt...@google.com> wrote: > Hi Gary and BCEL maintainers, > > My OutOfMemoryError problem that motivated BCEL-317 > <https://issues.apache.org/jira/browse/BCEL-317> ticket has been resolved > by a custom ClassPathRepository and I'm thinking to contribute this > solution to BCEL library: > > BCEL-320 <https://issues.apache.org/jira/browse/BCEL-320> A new > ClassPathRepository that can scan many JAR files without OutOfMemoryError > > > Test cases to reproduce OutOfMemoryError: > https://github.com/suztomo/bcel-oome-example > > What do you think? > > Regards, > Tomo > > On Wed, May 22, 2019 at 11:23 AM Tomo Suzuki <suzt...@google.com> wrote: > >> 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 >> > > > -- > Regards, > Tomo > -- Regards, Tomo
