I got a pre-review for 6912064 in January. These changes still work well; I just used them successfully with a performance tuning experiment.

There was a corner case bug which required an additional 'stopped()' check in graphKit. Here's the update:
http://cr.openjdk.java.net/~jrose/6912064/webrev.02/

The four lines in this file starting "// Profile disagrees with this path." are new:
http://cr.openjdk.java.net/~jrose/6912064/webrev.02/src/share/vm/opto/graphKit.cpp.udiff.html

That is the only difference from January.

Any further comments on this change set?

-- John

FTR, here is the previous communication. (Yes, it's been a while.)

From: John Rose <John.Rose at Sun.COM>
Date: January 26, 2010 7:14:13 PM PST
To: Vladimir Kozlov <Vladimir.Kozlov at Sun.COM>
Cc: Igor Veresov <Igor.Veresov at Sun.COM>, hotspot-compiler-dev compiler <hotspot-compiler-dev at openjdk.java.net>
Subject: Re: for pre-review (M): 6912064: type profiles need to be more thorough for dynamic language support
On Jan 26, 2010, at 6:59 PM, Vladimir Kozlov wrote:

Then why you generate the klass check with uncommon trap for instanceof
for cases when profiling shows that it will fail?
For code like this:

if (x instanceof String)
doString((String) x);
else if (x instanceof Number)
doNumber((Number) x);

If the 'x' is always an Integer, I want to fold all the tests up from the first instanceof, not the second.

The profile will show when 'x' is of a monomorphic type. That is more interesting than whether the first check happens to go true or false.

-- John

Search Discussions

  • Vladimir Kozlov at Jul 16, 2010 at 9:58 am
    Actually it was not last mail, I replied:

    Vladimir Kozlov wrote:
    I see, I missed that for instanceof case you just compare object's klass
    with profiled klass and don't check that it is subtype of require_klass
    (which is NULL for this case). Fine.
    About last changes. Why you not do 'stopped()' check after that call?:

    2668 cast_obj = maybe_cast_profiled_receiver(not_null_obj, data, tk->klass());

    Vladimir

    John Rose wrote:
    I got a pre-review for 6912064 in January. These changes still work well; I just used them successfully with a performance tuning experiment.

    There was a corner case bug which required an additional 'stopped()' check in graphKit. Here's the update:
    http://cr.openjdk.java.net/~jrose/6912064/webrev.02/

    The four lines in this file starting "// Profile disagrees with this path." are new:
    http://cr.openjdk.java.net/~jrose/6912064/webrev.02/src/share/vm/opto/graphKit.cpp.udiff.html

    That is the only difference from January.

    Any further comments on this change set?

    -- John

    FTR, here is the previous communication. (Yes, it's been a while.)

    From: John Rose <John.Rose at Sun.COM>
    Date: January 26, 2010 7:14:13 PM PST
    To: Vladimir Kozlov <Vladimir.Kozlov at Sun.COM>
    Cc: Igor Veresov <Igor.Veresov at Sun.COM>, hotspot-compiler-dev compiler <hotspot-compiler-dev at openjdk.java.net>
    Subject: Re: for pre-review (M): 6912064: type profiles need to be more thorough for dynamic language support
    On Jan 26, 2010, at 6:59 PM, Vladimir Kozlov wrote:

    Then why you generate the klass check with uncommon trap for instanceof
    for cases when profiling shows that it will fail?
    For code like this:

    if (x instanceof String)
    doString((String) x);
    else if (x instanceof Number)
    doNumber((Number) x);

    If the 'x' is always an Integer, I want to fold all the tests up from the first instanceof, not the second.

    The profile will show when 'x' is of a monomorphic type. That is more interesting than whether the first check happens to go true or false.

    -- John
  • John Rose at Jul 16, 2010 at 11:56 am

    On Jul 16, 2010, at 9:58 AM, Vladimir Kozlov wrote:

    About last changes. Why you not do 'stopped()' check after that call?:

    2668 cast_obj = maybe_cast_profiled_receiver(not_null_obj, data, tk->klass());
    Because the code (in gen_checkcast) works correctly in the case of dead control after the profile cast. When the obj_path control is dead, the null path might still be live, and the region/phi collect the right results in any case.

    Should I add a comment? (It's not the only way in which the two routines fail to be parallel, BTW.)

    Or, the other way to fix it might be to have load_object_klass and gen_subtype_check DTRT with dead control, and remove the explicit check in gen_instanceof. I took the path of least resistance.

    -- John
  • Vladimir Kozlov at Jul 16, 2010 at 11:56 am
    OK.

    Vladimir

    John Rose wrote:
    On Jul 16, 2010, at 9:58 AM, Vladimir Kozlov wrote:

    About last changes. Why you not do 'stopped()' check after that call?:

    2668 cast_obj = maybe_cast_profiled_receiver(not_null_obj, data, tk->klass());
    Because the code (in gen_checkcast) works correctly in the case of dead control after the profile cast. When the obj_path control is dead, the null path might still be live, and the region/phi collect the right results in any case.

    Should I add a comment? (It's not the only way in which the two routines fail to be parallel, BTW.)

    Or, the other way to fix it might be to have load_object_klass and gen_subtype_check DTRT with dead control, and remove the explicit check in gen_instanceof. I took the path of least resistance.

    -- John
  • John Rose at Aug 20, 2010 at 11:48 pm
    FYI: I attempted to commit this last month; the jprt sync failed and I timed out for the Summit.

    I just started a jprt job to integrate this change...

    -- John
    On Jul 16, 2010, at 11:56 AM, Vladimir Kozlov wrote:

    OK.

    Vladimir

    John Rose wrote:
    On Jul 16, 2010, at 9:58 AM, Vladimir Kozlov wrote:
    About last changes. Why you not do 'stopped()' check after that call?:

    2668 cast_obj = maybe_cast_profiled_receiver(not_null_obj, data, tk->klass());
    Because the code (in gen_checkcast) works correctly in the case of dead control after the profile cast. When the obj_path control is dead, the null path might still be live, and the region/phi collect the right results in any case.
    Should I add a comment? (It's not the only way in which the two routines fail to be parallel, BTW.)
    Or, the other way to fix it might be to have load_object_klass and gen_subtype_check DTRT with dead control, and remove the explicit check in gen_instanceof. I took the path of least resistance.
    -- John

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouphotspot-compiler-dev @
categoriesopenjdk
postedJul 15, '10 at 8:36p
activeAug 20, '10 at 11:48p
posts5
users2
websiteopenjdk.java.net

2 users in discussion

John Rose: 3 posts Vladimir Kozlov: 2 posts

People

Translate

site design / logo © 2021 Grokbase