RFR 8230162: ScopeImpl.remove() has O(N) performance
Jan Lahoda
jan.lahoda at oracle.com
Tue Oct 8 15:08:51 UTC 2019
On 08. 10. 19 17:06, Liam Miller-Cushon wrote:
> I'm happy to help push the patch once it's been reviewed, but I'm not a
> Reviewer.
>
> +Jan Lahoda <mailto:jan.lahoda at oracle.com>, does version .00 look OK to you?
I did actually push that:
http://hg.openjdk.java.net/jdk/jdk/rev/03165abce4cc
Jan
>
> On Tue, Oct 8, 2019 at 1:34 AM Ron Shapiro <ronshapiro at google.com
> <mailto:ronshapiro at google.com>> wrote:
>
> Hi, can someone sponsor/submit (I think that's right terminology)
> this patch? I don't want it to get lost.
>
> Here is the latest webrev for reference:
> http://cr.openjdk.java.net/~ronsh/8230162/webrev.01/
>
> On Mon, Sep 30, 2019 at 7:16 PM Brad Corso <bcorso at google.com
> <mailto:bcorso at google.com>> wrote:
>
> Friendly ping.
>
> On Wed, Sep 18, 2019 at 9:55 AM Brad Corso <bcorso at google.com
> <mailto:bcorso at google.com>> wrote:
>
> Hi Jan,
>
> I'm okay with either version so I'll leave that decision up
> to you.
>
> Let me know if there's anything else you need from me.
>
> Thanks!
> Brad
>
> On Wed, Sep 18, 2019 at 12:38 AM Jan Lahoda
> <jan.lahoda at oracle.com <mailto:jan.lahoda at oracle.com>> wrote:
>
> I've added logging to count how many Entries are
> created, and there's a
> little above 500000 instances created for java.base and
> a little less
> for java.desktop. So if the Entry would be 8 bytes
> bigger, it would be
> about 4MB, which does not sound terrible. So maybe we
> should go with
> version .00.
>
> Jan
>
> On 18. 09. 19 3:10, Brad Corso wrote:
> > Hi Jan, are you okay moving forward with
> > http://cr.openjdk.java.net/~ronsh/8230162/webrev.01/?
> >
> > On Wed, Sep 4, 2019 at 5:15 AM Ron Shapiro
> <ronshapiro at google.com <mailto:ronshapiro at google.com>
> > <mailto:ronshapiro at google.com
> <mailto:ronshapiro at google.com>>> wrote:
> >
> > Here's the updated webrev that Brad mentioned in
> his last message:
> > http://cr.openjdk.java.net/~ronsh/8230162/webrev.01/
> >
> > On Wed, Aug 28, 2019 at 2:40 AM Brad Corso
> <bcorso at google.com <mailto:bcorso at google.com>
> > <mailto:bcorso at google.com
> <mailto:bcorso at google.com>>> wrote:
> >
> > I believe "Do you have any estimates of
> the increase in size
> > in typical
> > usage, due to the extra field in Scope?"
> (Jon)
> >
> > I was seeing noticeably bad performance once
> the size of the
> > Entry.sibling linked list reached ~10000, and
> the max I saw was
> > ~30000 in a single scope. Given that an
> additional reference
> > adds 32/64b, this could add up to 120/240Kb
> for the cases I saw.
> >
> > I'd add, is there a chance to get an
> improvement in
> > Scope.remove speed
> > without making ScopeImpl.Entry bigger
> (assuming it gets
> > bigger(?))? One
> > possibility that occurred to me is that
> we could try not to
> > remove the
> > things from elems, but only mark them as
> removed. We would
> > need to do
> > filtering (and possibly the actual
> removal) while reading
> > from the Scope
> > (in getSymbols), so this is a different
> kind of trade-off.
> >
> > (Overall, I guess the question is whether
> we are trading
> > problems with
> >
> > Scope.remove speed in some cases for
> out-of-memory problems
> > in other cases.)
> >
> > Thanks, I've verified your suggestion also
> gives us the
> > performance improvements, so this change is
> okay with me.
> >
> >
> >
> > On Tue, Aug 27, 2019 at 12:05 AM Jan Lahoda
> > <jan.lahoda at oracle.com
> <mailto:jan.lahoda at oracle.com>
> <mailto:jan.lahoda at oracle.com
> <mailto:jan.lahoda at oracle.com>>> wrote:
> >
> > On 27. 08. 19 0:06, Brad Corso wrote:
> > > Sorry, what's the question?
> >
> > I believe "Do you have any estimates of
> the increase in size
> > in typical
> > usage, due to the extra field in Scope?"
> (Jon)
> >
> > I'd add, is there a chance to get an
> improvement in
> > Scope.remove speed
> > without making ScopeImpl.Entry bigger
> (assuming it gets
> > bigger(?))? One
> > possibility that occurred to me is that
> we could try not to
> > remove the
> > things from elems, but only mark them as
> removed. We would
> > need to do
> > filtering (and possibly the actual
> removal) while reading
> > from the Scope
> > (in getSymbols), so this is a different
> kind of trade-off.
> >
> > (Overall, I guess the question is whether
> we are trading
> > problems with
> > Scope.remove speed in some cases for
> out-of-memory problems
> > in other cases.)
> >
> > Jan
> >
> > >
> > > On Mon, Aug 26, 2019 at 1:54 PM Ron
> Shapiro
> > <ronshapiro at google.com
> <mailto:ronshapiro at google.com>
> <mailto:ronshapiro at google.com
> <mailto:ronshapiro at google.com>>
> > > <mailto:ronshapiro at google.com
> <mailto:ronshapiro at google.com>
> > <mailto:ronshapiro at google.com
> <mailto:ronshapiro at google.com>>>> wrote:
> > >
> > > Adding Brad back in to the thread
> since he would know
> > best
> > >
> > > בתאריך יום ב׳, 26 באוג׳ 2019,
> 19:40, מאת Jonathan Gibbons
> > > <jonathan.gibbons at oracle.com
> <mailto:jonathan.gibbons at oracle.com>
> > <mailto:jonathan.gibbons at oracle.com
> <mailto:jonathan.gibbons at oracle.com>>
> > <mailto:jonathan.gibbons at oracle.com
> <mailto:jonathan.gibbons at oracle.com>
> > <mailto:jonathan.gibbons at oracle.com
> <mailto:jonathan.gibbons at oracle.com>>>>:
> > >
> > >
> > > On 8/26/19 9:12 AM, Ron
> Shapiro wrote:
> > > >
> > > > Note that the patch was
> prepared by my
> > coworker, Brad (cc'd).
> > > I wasn't
> > > > sure what to do to make
> sure that he was
> > attributed correctly.
> > >
> > >
> > > Mention this when you have a
> sponsor to push the
> > changeset, so
> > > that it
> > > can be marked with
> "Contributed-By:"
> > >
> > > -- Jon
> > >
> >
>
More information about the compiler-dev
mailing list