commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tomo Suzuki <suzt...@google.com.INVALID>
Subject Re: [bcel] LRU Cache for ClassPathRepository
Date Mon, 08 Jul 2019 16:04:37 GMT
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 <suztomo@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 <suztomo@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 <suztomo@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 <suztomo@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 <suztomo@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 <garydgregory@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
>>>>> <suztomo@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 <
>>>>> garydgregory@gmail.com>
>>>>> > > wrote:
>>>>> > >
>>>>> > > > On Thu, May 2, 2019 at 10:59 AM Tomo Suzuki
>>>>> <suztomo@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 <
>>>>> garydgregory@gmail.com>
>>>>> > > > > wrote:
>>>>> > > > >
>>>>> > > > >> On Wed, May 1, 2019 at 6:37 PM Tomo Suzuki
>>>>> <suztomo@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
<
>>>>> garydgregory@gmail.com>
>>>>> > > > >> wrote:
>>>>> > > > >> >
>>>>> > > > >> > > Why not use getCachedInstance() ?
>>>>> > > > >> > >
>>>>> > > > >> > > Gary
>>>>> > > > >> > >
>>>>> > > > >> > > On Wed, May 1, 2019 at 5:20 PM Tomo
Suzuki
>>>>> > > > <suztomo@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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message