On 2/13/15 2:28 PM, Brendan Gregg wrote:
G'Day Serguei,

On Thu, Jan 15, 2015 at 1:26 PM, serguei.spit...@oracle.com <mailto:serguei.spit...@oracle.com> <serguei.spit...@oracle.com <mailto:serguei.spit...@oracle.com>> wrote:

    On 1/15/15 3:13 AM, Bertrand Delsart wrote:

        On 14/01/2015 20:12, John Rose wrote:

            On Jan 14, 2015, at 6:42 AM, Bertrand Delsart
            <bertrand.dels...@oracle.com
            <mailto:bertrand.dels...@oracle.com>
            <mailto:bertrand.dels...@oracle.com
            <mailto:bertrand.dels...@oracle.com>>> wrote:


                I would not prevent the JITs from using RBP as long as
                the changeset
                is not sufficient to guarantee the profiling will
                work... and IMHO
                solving the JSR292 issue will be much more intrusive
                (impacting
                HotSpot stack walking code).


            Here are some thoughts on that.

            SPARC uses L7 (L7_mh_SP_save) for the same purpose of
            method handle
            support as x86 uses RBP (rbp_mh_SP_save).  So there's not
            a hard
            requirement for x86 to take over RBP.

            (Deep background:  This purpose, in method handle support,
            is to allow
            an adapter to make changes to the caller's SP. The adapter
            is the
            initial callee from the caller, but may change argument
            shape, and
            tail-calls the ultimate callee.  Because it is a
            tail-call, the original
            caller must have a spot where his original SP can be
            preserved. The
            preservation works because the original caller knows he is
            calling a
            MH.invoke method, which requires the extra argument
            preservation.  The
            repertoire of argument shape changes is quite small,
            actually; it is not
            a very general mechanism since the LF machinery was put
            in. Perhaps the
            whole thing could be removed somehow, by finding
            alternative techniques
            for the few remaining changes.  OTOH, this SP-restoring
            mechanism may be
            helpful in doing more a general tail-call mechanism, and
            perhaps in
            managing int/comp mode changes more cleanly, so I'd like
            us to keep it.
              And document it better.)

            Any register or stack slot will do for this purpose, as
            long as (i) its
            value can be recovered after the MH.invoke call returns to
            the caller,
            and (ii) its value can be dug up somehow during stack
            walking. There
            are only a couple of places where stack walking code needs
            to sample the
            value, so they should be adjustable.

            Both x86 and SPARC use registers which are callee-save (or
            "non-volatile
            across calls") which satisfy properties (i) and (ii).  A
            standard stack
            slot (addressed based on caller's RBP) would probably also
            satisfy those
            properties.

            A variably-positioned stack slot would also work, which
            would require
            registering the position in each CodeBlob. That's
            unpleasant extra
            detail, but it would align somewhat with the current logic
            which allows
            each CodeBlob (nmethod, actually) to advertise which call
            sites need the
            special processing (see the function
            is_method_handle_return(caller_pc)).

            I recommend reserving a dead word of space in every stack
            frame that
            makes MH.invoke calls, at a fixed position relative to
            that frame's RBP.

            — John


        I perfectly agree that it is doable (and with your proposed
        approach).

        I just wanted to be sure people were aware that the RFE is
        more complex than what the current changeset may suggest. We
        are not just taking about reviewing and integrating a complete
        changeset contributed by the community. There is more work
        needed, either by the community or by Oracle. This will
        require changes at least in C1 and C2 call sequences, in the
        stack walking, in the creation and sizing of compiled frames...


    Just want to note about the stack walking...
    It also impacts some places that people are normally unaware of:
     - SA-based stack walking (jstack utility)
     - Solaris-specific: jhelper.d (dtrace jstack action support) and
    libjvm_db.so (pstack utility support)


Were these broken by the reuse of RBP as well? (I suspect so; I never tested my RBP fix with DTrace jstack()).

I'd not be surprised if these are broken while there is a chance they are not. My point is that the impact is bigger than normally expected and so, need to be tested.
We do not have yet good automated tests for jhelper.d and libjvm_db.so.

Thanks,
Serguei


Brendan



Reply via email to