Hi Gary and BCEL maintainers, (Thank you for merging PR #28) I created PR #29 <https://github.com/apache/commons-bcel/pull/29> for BCEL-321 <https://issues.apache.org/jira/browse/BCEL-321>: "Subclasses of ClassPathRepository only differ by its underlying cache?" I appreciate if you can review the PR when you have time.
Regards, Tomo On Thu, Jun 20, 2019 at 9:29 AM Tomo Suzuki <suzt...@google.com> wrote: > 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 > -- Regards, Tomo
