RFR: JDK-8256725: Metaspace: better blocktree and binlist asserts
Aleksey Shipilev
shade at openjdk.java.net
Fri Nov 20 10:34:07 UTC 2020
On Fri, 20 Nov 2020 08:34:18 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:
> Hi,
>
> may I have reviews please for this small change.
>
> To analyze JDK-8256572, I'd like to see more information in asserts for binlist and blocktree.
>
> This patch:
>
> - beefs up assertion messages when verifying binlist and blocktree
> - adds a canary to the blocktree node to detect overwriters
> - improves the blocktree printing
> - adds a gtest death test to test the overwrite detection.
>
> Thanks!
Stylistic drive-by comments.
src/hotspot/share/memory/metaspace/binList.hpp line 189:
> 187: const size_t s = MinWordSize + i;
> 188: int pos = 0;
> 189: for (Block* b = _blocks[i]; b != NULL; b = b->_next, pos ++) {
`pos++` seems to be more in-style here.
src/hotspot/share/memory/metaspace/blockTree.cpp line 44:
> 42: ": canary: " INTPTR_FORMAT \
> 43: ", parent " PTR_FORMAT \
> 44: ", left " PTR_FORMAT \
So, is this message with ":" (like `canary:`), or without it (like all other fields)?
src/hotspot/share/memory/metaspace/blockTree.cpp line 55:
> 53: p2i((n) ? (n)->_left : NULL), \
> 54: p2i((n) ? (n)->_right : NULL), \
> 55: p2i((n) ? (n)->_next : 0), \
This is still pointer, right, judging by `p2i`? So it should return `NULL` in the ternary op?
src/hotspot/share/memory/metaspace/blockTree.cpp line 63:
> 61:
> 62: // This assert prints the tree, then stops (generic message)
> 63: #define assrt0(cond, format, ...) \
The names get hard to read. I thought it was as typo initially :) Suggestion: `tree_assert`?
src/hotspot/share/memory/metaspace/blockTree.cpp line 89:
> 87: // assert a condition with information about node "n"
> 88: #define assrt0n(cond, failure_node) assrt0(cond, "Invalid node: " NODE_FORMAT, NODE_FORMAT_ARGS(failure_node))
> 89:
Do you think this assert is only usable in this method? I would probably be cleaner to move it next to the assertion it chains with.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1339
More information about the hotspot-runtime-dev
mailing list