RFR: 8049229 sun/tools/jps/jps-l_1.sh fails with Execution failed: exit code 1
bug: https://bugs.openjdk.java.net/browse/JDK-8049229 webrev: http://cr.openjdk.java.net/~sla/8049229/webrev.00/ The problem here is that the awk scripts in the test match the lines in the ups output that end in “.jar twice and end up failing. The solution is to stop matching after we have found one match. Thanks, /Staffan
Re: RFR: 8049229 sun/tools/jps/jps-l_1.sh fails with Execution failed: exit code 1
Looks good for me. On 2014-07-03 13:22, Staffan Larsen wrote: bug: https://bugs.openjdk.java.net/browse/JDK-8049229 webrev: http://cr.openjdk.java.net/~sla/8049229/webrev.00/ The problem here is that the awk scripts in the test match the lines in the ups output that end in “.jar twice and end up failing. The solution is to stop matching after we have found one match. Thanks, /Staffan -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
Re: RFR(M): 8039905: heapdump/OnOOMToFile and heapdump/OnOOMToPath fail with assert(fr().interpreter_frame_expression_stack_size() = length) failed: error in expression stack!
Markus,
I've done another pass through the webrev.
It looks good in general.
Thank you for fixing it!
As for the ::locals() and ::expressions() refactoring, my guess is that
your motivation was
to make it closer to what the GC is doing. Is it true?
You can keep it as you like because my comment is not a big or strong
statement.
It is just my personal preference.
But, the refactoring made it harder to review the changes as the
original mapping has gone.
As we privately discussed, this code may need further improvements
related to the dead locals
and so, we may need to open new bugs for recognized issues.
Also, It seems you are right on the meaning of the T_CONFLICT that it is
set for the dead locals.
However, in my bug/testcase context the T_CONFLICT value was not set for
dead local.
The observed value of the dead local was T_INT (as uninitialized data)
which looks pretty strange.
It is possible, we have a deal with another bug here.
Thanks,
Serguei
On 7/2/14 10:04 PM, serguei.spit...@oracle.com wrote:
Hi Markus,
Sorry for the latency.
I hope this review is still needed.
src/share/vm/interpreter/oopMapCache.hpp
A minor comment:
The function blocks indentation was originally aligned, but this fix
partially broke it:
- uintptr_t entry_at(int offset){ int i = offset * bits_per_entry;
return bit_mask()[i / BitsPerWord] (i % BitsPerWord); }
+ uintptr_t entry_at(int offset) const { int i = offset * bits_per_entry;
return bit_mask()[i / BitsPerWord] (i % BitsPerWord); }
void set_expression_stack_size(int sz){ _expression_stack_size = sz; }
#ifdef ENABLE_ZAP_DEAD_LOCALS
- bool is_dead(int offset) { return (entry_at(offset) (1
dead_bit_number)) != 0; }
+ bool is_dead(int offset) const{ return (entry_at(offset) (1
dead_bit_number)) != 0; }
#endif
// Lookup
- bool match(methodHandle method, int bci) { return _method == method()
_bci == bci; }
- bool is_empty();
+ bool match(methodHandle method, int bci) const { return _method == method()
_bci == bci; }
+ bool is_empty() const;
||
src/share/vm/runtime/vframe.cpp
The implementation of the ::stack_data () combined the logics from
::locals() and ::expressions().
As a result, the function became unreasonably more complex to have a
deal with this combination.
The original approach looks better even though some fragments are
repeated twice.
In other words, more simple and flat lines is better than less lines
with more complexity
as it adds another dimension to track down.
I need a little bit more time to better understand the vframe.cpp part
of the fix.
Thanks,
Serguei
On 6/25/14 4:58 AM, Markus Grönlund wrote:
Greetings,
Kindly looking for reviews for the following change:
Bug: http://bugs.openjdk.java.net/browse/JDK-8039905
Webrev: http://cr.openjdk.java.net/~mgronlun/8039905/webrev01/
http://cr.openjdk.java.net/%7Emgronlun/8039905/webrev01/
Description:
JVMTI inspection code for following references makes use of a
VM_HeapWalkOperation in order to follow-up root sets.
In bug:
https://bugs.openjdk.java.net/browse/JDK-8038624 -
interpretedVFrame::expressions() must respect InterpreterOopMap for
liveness, it was found that the interpretedVFrame code had a
discrepancy between basing length information from both asking the
interpreter frame (which saw live expression slots for calls
instructions) as well as the oop map (which did not).
The liveness decisions for a particular BCI should be based on what
the oopmap gives, and that was done as of that bug (8038624).
In that process, I added an assert in an attempt to validate certain
assumptions, and this assert has triggered during nightly testing in
some cases.
I have therefore inspected the GC code which follow-up roots from an
interpreted frame (please see frame::oops_interpreted_do() and
InterpreterFrameClosure::do_offset() (in frame.cpp) for reference),
and reworked InterpretedVFrame so that inspections for the locals
and expression slots are done in the same way as the GC code
(especially in regards to taking decisions on the InterpreterOopMap).
I needed to use InterpreterOopMap from a const context, and this is
why I have constified this class where needed, as well as making
number_of_entries() a const public accessor (to easily reach
oop_mask length info).
Testing completed:
nsk/jdi* tests (especially the problematic ones reported for 8039905)
compiler/6507107/HeapWalkingTest
Thanks
Markus
Re: RFR(M): 8028474: sun/jvmstat/monitor/MonitoredVm/MonitorVmStartTerminate.sh timeout, leaves looping process behind
Hi Roger, The test has been updated. It now uses System.nanoTime. http://cr.openjdk.java.net/~egahlin/8028474_6/ Thanks Erik roger riggs skrev 2014-07-01 14:35: Hi Erik, Consider switching to System.nanoTime; it is not sensitive to clock changes and avoids leaving a land mine that may cause a spurious non-repeatable test failure. 'Deducing it from the log' means there is a failure and creates probably an hour or two of work for some quality engineer and burns a couple of hours re-running the test run. Roger On 7/1/2014 3:37 AM, Erik Gahlin wrote: JavaProcess.waitForRemoval: How about using timestamps (currentTimeMillis()) before the loop and for each ite ration to determine if the timeout has expired (instead of time+=100”)? The code now uses currentTimeMillis(). Premature timeouts due to clock changes can be deduced from the log.
Re: RFR: 8049229 sun/tools/jps/jps-l_1.sh fails with Execution failed: exit code 1
Looks fine. Those simple file names really look like FQNs ... -JB- On 07/03/2014 11:22 AM, Staffan Larsen wrote: bug: https://bugs.openjdk.java.net/browse/JDK-8049229 webrev: http://cr.openjdk.java.net/~sla/8049229/webrev.00/ The problem here is that the awk scripts in the test match the lines in the ups output that end in “.jar twice and end up failing. The solution is to stop matching after we have found one match. Thanks, /Staffan
Re: RFR(M): 8028474: sun/jvmstat/monitor/MonitoredVm/MonitorVmStartTerminate.sh timeout, leaves looping process behind
Hi Erik, nice cleanup! L160 break statement after this line would save unnecessary iterating when a process has already been found L172 You could use return true. - 'MonitoredVmUtil.mainArgs(target).contains(args)' has been asserted to be true on L171 -JB- On 07/03/2014 01:06 PM, Erik Gahlin wrote: Hi Roger, The test has been updated. It now uses System.nanoTime. http://cr.openjdk.java.net/~egahlin/8028474_6/ Thanks Erik roger riggs skrev 2014-07-01 14:35: Hi Erik, Consider switching to System.nanoTime; it is not sensitive to clock changes and avoids leaving a land mine that may cause a spurious non-repeatable test failure. 'Deducing it from the log' means there is a failure and creates probably an hour or two of work for some quality engineer and burns a couple of hours re-running the test run. Roger On 7/1/2014 3:37 AM, Erik Gahlin wrote: JavaProcess.waitForRemoval: How about using timestamps (currentTimeMillis()) before the loop and for each ite ration to determine if the timeout has expired (instead of time+=100”)? The code now uses currentTimeMillis(). Premature timeouts due to clock changes can be deduced from the log.
Re: RFR(M): 8028474: sun/jvmstat/monitor/MonitoredVm/MonitorVmStartTerminate.sh timeout, leaves looping process behind
On 07/03/2014 04:44 PM, Erik Gahlin wrote: Hi Jaroslav, Here is an updated webrev: http://cr.openjdk.java.net/~egahlin/8028474_7/ Thanks. No more comments. -JB- See comments inline. Jaroslav Bachorik skrev 2014-07-03 16:03: Hi Erik, nice cleanup! L160 break statement after this line would save unnecessary iterating when a process has already been found I'm not a big fan of break statements in nested loops, so refactored out a separate method that returns when the process has been found. Also updated releaseStarted L172 You could use return true. - 'MonitoredVmUtil.mainArgs(target).contains(args)' has been asserted to be true on L171 Ops, not sure how it got there. Thanks Erik -JB- On 07/03/2014 01:06 PM, Erik Gahlin wrote: Hi Roger, The test has been updated. It now uses System.nanoTime. http://cr.openjdk.java.net/~egahlin/8028474_6/ Thanks Erik roger riggs skrev 2014-07-01 14:35: Hi Erik, Consider switching to System.nanoTime; it is not sensitive to clock changes and avoids leaving a land mine that may cause a spurious non-repeatable test failure. 'Deducing it from the log' means there is a failure and creates probably an hour or two of work for some quality engineer and burns a couple of hours re-running the test run. Roger On 7/1/2014 3:37 AM, Erik Gahlin wrote: JavaProcess.waitForRemoval: How about using timestamps (currentTimeMillis()) before the loop and for each ite ration to determine if the timeout has expired (instead of time+=100”)? The code now uses currentTimeMillis(). Premature timeouts due to clock changes can be deduced from the log.
RE: RFR(M): 8039905: heapdump/OnOOMToFile and heapdump/OnOOMToPath fail with assert(fr().interpreter_frame_expression_stack_size() = length) failed: error in expression stack!
Hi Serguei,
Thanks a lot for taking a look at this.
Yes, the primary motivation for changing this is to have this code do the
same as the GC in respect of InterpretedVFrame (granted, we do care about
value slots here, where GC doesn't).
Sorry about making this more complex, I have tried to make some of it a bit
easier to read based on your input, please take a look if you think this is
easier to follow:
New webrev suggestion: http://cr.openjdk.java.net/~mgronlun/8039905/webrev02/
And I agree with all the points about T_CONFLICT and dead locals that we need
to consider as a next step.
Thanks again for taking the time
Many thanks
Markus
From: Serguei Spitsyn
Sent: den 3 juli 2014 12:59
To: Markus Grönlund; hotspot-runtime-dev; Serviceability Dev
Subject: Re: RFR(M): 8039905: heapdump/OnOOMToFile and heapdump/OnOOMToPath
fail with assert(fr().interpreter_frame_expression_stack_size() = length)
failed: error in expression stack!
Markus,
I've done another pass through the webrev.
It looks good in general.
Thank you for fixing it!
As for the ::locals() and ::expressions() refactoring, my guess is that your
motivation was
to make it closer to what the GC is doing. Is it true?
You can keep it as you like because my comment is not a big or strong statement.
It is just my personal preference.
But, the refactoring made it harder to review the changes as the original
mapping has gone.
As we privately discussed, this code may need further improvements related to
the dead locals
and so, we may need to open new bugs for recognized issues.
Also, It seems you are right on the meaning of the T_CONFLICT that it is set
for the dead locals.
However, in my bug/testcase context the T_CONFLICT value was not set for dead
local.
The observed value of the dead local was T_INT (as uninitialized data) which
looks pretty strange.
It is possible, we have a deal with another bug here.
Thanks,
Serguei
On 7/2/14 10:04 PM, HYPERLINK
mailto:serguei.spit...@oracle.comserguei.spit...@oracle.com wrote:
Hi Markus,
Sorry for the latency.
I hope this review is still needed.
src/share/vm/interpreter/oopMapCache.hpp
A minor comment:
The function blocks indentation was originally aligned, but this fix partially
broke it:
- uintptr_t entry_at(int offset) { int i = offset * bits_per_entry;
return bit_mask()[i / BitsPerWord] (i % BitsPerWord); }
+ uintptr_t entry_at(int offset) const { int i = offset *
bits_per_entry; return bit_mask()[i / BitsPerWord] (i % BitsPerWord); }
void set_expression_stack_size(int sz) { _expression_stack_size = sz; }
#ifdef ENABLE_ZAP_DEAD_LOCALS
- bool is_dead(int offset) { return (entry_at(offset)
(1 dead_bit_number)) != 0; }
+ bool is_dead(int offset) const { return (entry_at(offset)
(1 dead_bit_number)) != 0; }
#endif
// Lookup
- bool match(methodHandle method, int bci) { return _method == method()
_bci == bci; }
- bool is_empty();
+ bool match(methodHandle method, int bci) const { return _method == method()
_bci == bci; }
+ bool is_empty() const;
src/share/vm/runtime/vframe.cpp
The implementation of the ::stack_data () combined the logics from ::locals()
and ::expressions().
As a result, the function became unreasonably more complex to have a deal
with this combination.
The original approach looks better even though some fragments are repeated
twice.
In other words, more simple and flat lines is better than less lines with
more complexity
as it adds another dimension to track down.
I need a little bit more time to better understand the vframe.cpp part of the
fix.
Thanks,
Serguei
On 6/25/14 4:58 AM, Markus Grönlund wrote:
Greetings,
Kindly looking for reviews for the following change:
Bug: http://bugs.openjdk.java.net/browse/JDK-8039905
Webrev: HYPERLINK
http://cr.openjdk.java.net/%7Emgronlun/8039905/webrev01/http://cr.openjdk.java.net/~mgronlun/8039905/webrev01/
Description:
JVMTI inspection code for following references makes use of a
VM_HeapWalkOperation in order to follow-up root sets.
In bug:
https://bugs.openjdk.java.net/browse/JDK-8038624 -
interpretedVFrame::expressions() must respect InterpreterOopMap for liveness,
it was found that the interpretedVFrame code had a discrepancy between basing
length information from both asking the interpreter frame (which saw live
expression slots for calls instructions) as well as the oop map (which did
not).
The liveness decisions for a particular BCI should be based on what the oopmap
gives, and that was done as of that bug (8038624).
In that process, I added an assert in an attempt to validate certain
assumptions, and this assert has triggered during nightly testing in some
cases.
I have therefore inspected the GC code which follow-up roots from an
interpreted frame (please see frame::oops_interpreted_do() and
Re: RFR(M): 8028474: sun/jvmstat/monitor/MonitoredVm/MonitorVmStartTerminate.sh timeout, leaves looping process behind
I’m still ok with this. /Staffan On 3 jul 2014, at 16:42, Jaroslav Bachorik jaroslav.bacho...@oracle.com wrote: On 07/03/2014 04:44 PM, Erik Gahlin wrote: Hi Jaroslav, Here is an updated webrev: http://cr.openjdk.java.net/~egahlin/8028474_7/ Thanks. No more comments. -JB- See comments inline. Jaroslav Bachorik skrev 2014-07-03 16:03: Hi Erik, nice cleanup! L160 break statement after this line would save unnecessary iterating when a process has already been found I'm not a big fan of break statements in nested loops, so refactored out a separate method that returns when the process has been found. Also updated releaseStarted L172 You could use return true. - 'MonitoredVmUtil.mainArgs(target).contains(args)' has been asserted to be true on L171 Ops, not sure how it got there. Thanks Erik -JB- On 07/03/2014 01:06 PM, Erik Gahlin wrote: Hi Roger, The test has been updated. It now uses System.nanoTime. http://cr.openjdk.java.net/~egahlin/8028474_6/ Thanks Erik roger riggs skrev 2014-07-01 14:35: Hi Erik, Consider switching to System.nanoTime; it is not sensitive to clock changes and avoids leaving a land mine that may cause a spurious non-repeatable test failure. 'Deducing it from the log' means there is a failure and creates probably an hour or two of work for some quality engineer and burns a couple of hours re-running the test run. Roger On 7/1/2014 3:37 AM, Erik Gahlin wrote: JavaProcess.waitForRemoval: How about using timestamps (currentTimeMillis()) before the loop and for each ite ration to determine if the timeout has expired (instead of time+=100”)? The code now uses currentTimeMillis(). Premature timeouts due to clock changes can be deduced from the log.
RE: RFR(M): 8039905: heapdump/OnOOMToFile and heapdump/OnOOMToPath fail with assert(fr().interpreter_frame_expression_stack_size() = length) failed: error in expression stack!
Hi Coleen,
Thanks for taking a look. What about this?
http://cr.openjdk.java.net/~mgronlun/8039905/webrev03/
If you are ok i will push this suggestion.
/Markus
-Original Message-
From: Coleen Phillimore
Sent: den 3 juli 2014 19:33
To: hotspot-runtime-...@openjdk.java.net
Subject: Re: RFR(M): 8039905: heapdump/OnOOMToFile and heapdump/OnOOMToPath
fail with assert(fr().interpreter_frame_expression_stack_size() = length)
failed: error in expression stack!
Hi Markus,
I generally agree with Serguei about adding a conditional to have one function
do two different things, but in this case the less duplication
with the size calculations overrides this concern. I think your
refactoring improves this. I have two requests though. Can you make
350 StackValueCollection* interpretedVFrame::stack_data(bool expressions /*
false */) const {
The boolean not be a default parameter. Another request that I
normally make is that the bool be some sort of enum but since this isn't
widespread, I think this would be overkill.
The second is can you make this parameter not be a non-const reference.
289 static void stack_locals(const InterpreterOopMap oop_mask,
290 const frame fr,
291 int length,
292 int max_locals,
293 StackValueCollection result) {
also in stack_expressions().
You can't tell which parameter is being modified. You can't tell
anyway but passing *result in the caller looks weird. Maybe making
result the first parameter would also help show that that's what's being
modified.
These are minor style comments. I've verified the content makes sense.
Please check in if no further discussion is needed. Thank you for fixing this!
The gatekeepers will thank you too.
Thanks,
Coleen
On 7/3/14, 12:34 PM, Markus Grönlund wrote:
Hi Serguei,
Thanks a lot for taking a look at this.
Yes, the primary motivation for changing this is to have this code do the
same as the GC in respect of InterpretedVFrame (granted, we do care about
value slots here, where GC doesn't).
Sorry about making this more complex, I have tried to make some of it a bit
easier to read based on your input, please take a look if you think this is
easier to follow:
New webrev suggestion:
http://cr.openjdk.java.net/~mgronlun/8039905/webrev02/
And I agree with all the points about T_CONFLICT and dead locals that we need
to consider as a next step.
Thanks again for taking the time
Many thanks
Markus
From: Serguei Spitsyn
Sent: den 3 juli 2014 12:59
To: Markus Grönlund; hotspot-runtime-dev; Serviceability Dev
Subject: Re: RFR(M): 8039905: heapdump/OnOOMToFile and heapdump/OnOOMToPath
fail with assert(fr().interpreter_frame_expression_stack_size() = length)
failed: error in expression stack!
Markus,
I've done another pass through the webrev.
It looks good in general.
Thank you for fixing it!
As for the ::locals() and ::expressions() refactoring, my guess is
that your motivation was to make it closer to what the GC is doing. Is it
true?
You can keep it as you like because my comment is not a big or strong
statement.
It is just my personal preference.
But, the refactoring made it harder to review the changes as the original
mapping has gone.
As we privately discussed, this code may need further improvements
related to the dead locals and so, we may need to open new bugs for
recognized issues.
Also, It seems you are right on the meaning of the T_CONFLICT that it is set
for the dead locals.
However, in my bug/testcase context the T_CONFLICT value was not set for dead
local.
The observed value of the dead local was T_INT (as uninitialized data) which
looks pretty strange.
It is possible, we have a deal with another bug here.
Thanks,
Serguei
On 7/2/14 10:04 PM, HYPERLINK
mailto:serguei.spit...@oracle.comserguei.spit...@oracle.com wrote:
Hi Markus,
Sorry for the latency.
I hope this review is still needed.
src/share/vm/interpreter/oopMapCache.hpp
A minor comment:
The function blocks indentation was originally aligned, but this fix
partially broke it:
- uintptr_t entry_at(int offset){ int i = offset *
bits_per_entry; return bit_mask()[i / BitsPerWord] (i % BitsPerWord); }
+ uintptr_t entry_at(int offset) const { int i = offset *
bits_per_entry; return bit_mask()[i / BitsPerWord] (i % BitsPerWord); }
void set_expression_stack_size(int sz){ _expression_stack_size = sz; }
#ifdef ENABLE_ZAP_DEAD_LOCALS
- bool is_dead(int offset) { return (entry_at(offset)
(1 dead_bit_number)) != 0; }
+ bool is_dead(int offset) const{ return (entry_at(offset)
(1 dead_bit_number)) != 0; }
#endif
// Lookup
- bool match(methodHandle method, int bci) { return _method ==
method()
Re: RFR(M): 8039905: heapdump/OnOOMToFile and heapdump/OnOOMToPath fail with assert(fr().interpreter_frame_expression_stack_size() = length) failed: error in expression stack!
On 7/3/14, 2:38 PM, Markus Grönlund wrote:
Hi Coleen,
Thanks for taking a look. What about this?
http://cr.openjdk.java.net/~mgronlun/8039905/webrev03/
If you are ok i will push this suggestion.
Yes, I think this looks good. Ship it!
Thanks!
Coleen
/Markus
-Original Message-
From: Coleen Phillimore
Sent: den 3 juli 2014 19:33
To: hotspot-runtime-...@openjdk.java.net
Subject: Re: RFR(M): 8039905: heapdump/OnOOMToFile and heapdump/OnOOMToPath fail with
assert(fr().interpreter_frame_expression_stack_size() = length) failed: error in
expression stack!
Hi Markus,
I generally agree with Serguei about adding a conditional to have one function
do two different things, but in this case the less duplication
with the size calculations overrides this concern. I think your
refactoring improves this. I have two requests though. Can you make
350 StackValueCollection* interpretedVFrame::stack_data(bool expressions /*
false */) const {
The boolean not be a default parameter. Another request that I
normally make is that the bool be some sort of enum but since this isn't
widespread, I think this would be overkill.
The second is can you make this parameter not be a non-const reference.
289 static void stack_locals(const InterpreterOopMap oop_mask,
290 const frame fr,
291 int length,
292 int max_locals,
293 StackValueCollection result) {
also in stack_expressions().
You can't tell which parameter is being modified. You can't tell
anyway but passing *result in the caller looks weird. Maybe making
result the first parameter would also help show that that's what's being
modified.
These are minor style comments. I've verified the content makes sense.
Please check in if no further discussion is needed. Thank you for fixing this!
The gatekeepers will thank you too.
Thanks,
Coleen
On 7/3/14, 12:34 PM, Markus Grönlund wrote:
Hi Serguei,
Thanks a lot for taking a look at this.
Yes, the primary motivation for changing this is to have this code do the
same as the GC in respect of InterpretedVFrame (granted, we do care about value
slots here, where GC doesn't).
Sorry about making this more complex, I have tried to make some of it a bit
easier to read based on your input, please take a look if you think this is
easier to follow:
New webrev suggestion:
http://cr.openjdk.java.net/~mgronlun/8039905/webrev02/
And I agree with all the points about T_CONFLICT and dead locals that we need
to consider as a next step.
Thanks again for taking the time
Many thanks
Markus
From: Serguei Spitsyn
Sent: den 3 juli 2014 12:59
To: Markus Grönlund; hotspot-runtime-dev; Serviceability Dev
Subject: Re: RFR(M): 8039905: heapdump/OnOOMToFile and heapdump/OnOOMToPath fail with
assert(fr().interpreter_frame_expression_stack_size() = length) failed: error in
expression stack!
Markus,
I've done another pass through the webrev.
It looks good in general.
Thank you for fixing it!
As for the ::locals() and ::expressions() refactoring, my guess is
that your motivation was to make it closer to what the GC is doing. Is it true?
You can keep it as you like because my comment is not a big or strong statement.
It is just my personal preference.
But, the refactoring made it harder to review the changes as the original
mapping has gone.
As we privately discussed, this code may need further improvements
related to the dead locals and so, we may need to open new bugs for recognized
issues.
Also, It seems you are right on the meaning of the T_CONFLICT that it is set
for the dead locals.
However, in my bug/testcase context the T_CONFLICT value was not set for dead
local.
The observed value of the dead local was T_INT (as uninitialized data) which
looks pretty strange.
It is possible, we have a deal with another bug here.
Thanks,
Serguei
On 7/2/14 10:04 PM, HYPERLINK
mailto:serguei.spit...@oracle.comserguei.spit...@oracle.com wrote:
Hi Markus,
Sorry for the latency.
I hope this review is still needed.
src/share/vm/interpreter/oopMapCache.hpp
A minor comment:
The function blocks indentation was originally aligned, but this fix
partially broke it:
- uintptr_t entry_at(int offset){ int i = offset * bits_per_entry;
return bit_mask()[i / BitsPerWord] (i % BitsPerWord); }
+ uintptr_t entry_at(int offset) const { int i = offset * bits_per_entry;
return bit_mask()[i / BitsPerWord] (i % BitsPerWord); }
void set_expression_stack_size(int sz){ _expression_stack_size = sz; }
#ifdef ENABLE_ZAP_DEAD_LOCALS
- bool is_dead(int offset) { return (entry_at(offset) (1
dead_bit_number)) != 0; }
+ bool is_dead(int offset) const{ return (entry_at(offset) (1
dead_bit_number)) != 0; }
#endif
// Lookup
- bool match(methodHandle method,
RE: RFR(M): 8039905: heapdump/OnOOMToFile and heapdump/OnOOMToPath fail with assert(fr().interpreter_frame_expression_stack_size() = length) failed: error in expression stack!
Coleen, Serguei, Thanks for your reviews! Cheers Markus -Original Message- From: Coleen Phillimore Sent: den 3 juli 2014 21:24 To: Markus Grönlund Cc: hotspot-runtime-...@openjdk.java.net; Serviceability Dev Subject: Re: RFR(M): 8039905: heapdump/OnOOMToFile and heapdump/OnOOMToPath fail with assert(fr().interpreter_frame_expression_stack_size() = length) failed: error in expression stack! On 7/3/14, 2:38 PM, Markus Grönlund wrote: Hi Coleen, Thanks for taking a look. What about this? http://cr.openjdk.java.net/~mgronlun/8039905/webrev03/ If you are ok i will push this suggestion. Yes, I think this looks good. Ship it! Thanks! Coleen cut
