Issue | Fix Version | Assignee | Priority | Status | Resolution | Resolved In Build |
---|---|---|---|---|---|---|
JDK-8082619 | emb-9 | Bengt Rutisson | P3 | Resolved | Fixed | team |
The method CollectedHeap::is_in() has the following comment:
// Returns "TRUE" iff "p" points into the committed areas of the heap.
// Since this method can be expensive in general, we restrict its
// use to assertion checking only.
This is incorrect since we don't just use it for asserts. We also use it in os::print_location() which is in turn is used by several different code paths, such as verification, crash dumping and logging.
In GenCollectedHeap::is_in() there is an attempt to verify that we only call this method from places that are not performance critical. So, its implementation looks like this:
bool GenCollectedHeap::is_in(const void* p) const {
#ifndef ASSERT
guarantee(VerifyBeforeGC ||
VerifyDuringGC ||
VerifyBeforeExit ||
VerifyDuringStartup ||
PrintAssembly ||
tty->count() != 0 || // already printing
VerifyAfterGC ||
VMError::fatal_error_in_progress(), "too expensive");
#endif
return _young_gen->is_in(p) || _old_gen->is_in(p);
}
It is quite odd that we do this check here and even more strange that it is only done in GenCollectedHeap and not in the other implementations (G1CollectedHeap and ParallelScavengeHeap).
I doubt that this check is useful. The method is actually not that slow. It basically just iterates over the three spaces in young gen to do range checks and then does a single range check on the old gen. So, 4 range checks all together.
Rather than having GenCollectedHeap::is_in() know about all callers of it I think we should remove the guarantee.
// Returns "TRUE" iff "p" points into the committed areas of the heap.
// Since this method can be expensive in general, we restrict its
// use to assertion checking only.
This is incorrect since we don't just use it for asserts. We also use it in os::print_location() which is in turn is used by several different code paths, such as verification, crash dumping and logging.
In GenCollectedHeap::is_in() there is an attempt to verify that we only call this method from places that are not performance critical. So, its implementation looks like this:
bool GenCollectedHeap::is_in(const void* p) const {
#ifndef ASSERT
guarantee(VerifyBeforeGC ||
VerifyDuringGC ||
VerifyBeforeExit ||
VerifyDuringStartup ||
PrintAssembly ||
tty->count() != 0 || // already printing
VerifyAfterGC ||
VMError::fatal_error_in_progress(), "too expensive");
#endif
return _young_gen->is_in(p) || _old_gen->is_in(p);
}
It is quite odd that we do this check here and even more strange that it is only done in GenCollectedHeap and not in the other implementations (G1CollectedHeap and ParallelScavengeHeap).
I doubt that this check is useful. The method is actually not that slow. It basically just iterates over the three spaces in young gen to do range checks and then does a single range check on the old gen. So, 4 range checks all together.
Rather than having GenCollectedHeap::is_in() know about all callers of it I think we should remove the guarantee.
- backported by
-
JDK-8082619 Remove guarantee from GenCollectedHeap::is_in()
-
- Resolved
-