Request for reviews (M): 6873116: Modify reexecute implementation to use pcDesc to record the reexecute bit
Vladimir.Kozlov at Sun.COM
Wed Aug 19 15:17:17 PDT 2009
I talked with Changpeng about this.
I think lazy objects decoding will needs more work and testing.
Can Changpeng push his changes as it is and I will file new
RFI for lazy decoding and fix it? After that we will be able
reduce ScopeDesc constructors.
Tom Rodriguez wrote:
> On Aug 19, 2009, at 1:56 PM, changpeng fang - Sun Microsystems - Santa
> Clara United States wrote:
>> On 08/19/09 13:26, Tom Rodriguez wrote:
>>> This looks good. The SA fixes look right as well. The only
>>> suggestion I might make is to pass the PcDesc into the ScopeDesc
>>> constructor instead of passing in each of the arguments. It's ok the
>>> way it is but the arguments are just a mirror of PcDesc.
>> In way, the two constructors will be reduced to one. Can I use a
>> default argument to specify
>> "serialized_null" considering "avoid a .hpp-.hpp dependency" comment?
> The circularity is caused by SimpleScopeDesc which seems like variant of
> limited usefulness. I don't really understand the need for two
> ScopeDesc constructors here in the first place. The only difference
> between them is whether they decode the objects or not and that should
> probably be lazy, which would remove any difference in them. The second
> constructor is only used when decoding line numbers for JVMTI which
> seems like a strange place to worry about decoding the objects. Normal
> stack walking would probably prefer to skip this step too. Switching to
> lazy object decoding and having a single constructor taking a PcDesc
> seems better to me. I think we could get rid of SimpleScopeDesc too.
> It's only used in one place, nmethod::preserve_callee_argument_oops,
> which is only called in the rare instance where a GC is requested while
> a call site is being resolved. The performance difference between using
> a normal ScopeDesc and SimpleScopeDesc would be pretty minimal.
>> // Constructor
>> ScopeDesc(const nmethod* code, int decode_offset, int
>> obj_decode_offset, bool reexecute);
>> // Calls above, giving default value of "serialized_null" to the
>> // "obj_decode_offset" argument. (We don't use a default argument to
>> // avoid a .hpp-.hpp dependency.)
>> ScopeDesc(const nmethod* code, int decode_offset, bool reexecute);
>>> By the way, does anyone know why the objects for EA are decoded
>>> eagerly instead of being decoded lazily like all the other debug
>>> info? That seems like unneeded overhead for most uses of ScopeDesc
>>> for stack walking.
>>> On Aug 19, 2009, at 10:55 AM, changpeng fang - Sun Microsystems -
>>> Santa Clara United States wrote:
>>>> Fixed 6873116: Modify reexecute implementation to use pcDesc to
>>>> record the reexecute bit
>>>> The implementation in this work is mostly from Christian's work on
>>>> reexecute/mh bit changes.
>>>> Current implementation of reexecute bit uses DebugInfoStreams to
>>>> record the reexecute bit by compressing the reexecute bit
>>>> into bci field. This makes the code look ugly
>>>> (e.g.read_bci_and_reexecute(bool& reexecute). It also makes the
>>>> future addition
>>>> of debuginfo difficult.
>>>> In this work, we use pcDesc to record the reexecute bit obtained
>>>> from describe_scope, and then store the reexecute bit to
>>>> scopeDesc when a scopeDesc object is created.
>>>> JPRT, CompileTheWord, refworkload
More information about the hotspot-compiler-dev