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] Idea to share ConstantUtf8 of same value among JavaClass instances
Date Wed, 22 May 2019 15:23:34 GMT
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

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