feat: more backend capabilities for heapdump #233
Conversation
| static { | ||
| ARGS.put("queryString", OQL); | ||
| JDK_MANAGED_BUFFER_ARGS.put("queryString", JDK_MANAGED_BUFFER_OQL); |
There was a problem hiding this comment.
What about?
static final Map<String, Object> JDK_MANAGED_BUFFER_ARGS = Collections.singletonMap("queryString", ...);
or even with a helper:
static final Map<String, Object> JDK_MANAGED_BUFFER_ARGS = makeQueryMap(oql);
| queryGc.put("queryString", "SELECT toString(g.name) AS GC FROM sun.management.GarbageCollectorImpl g"); | ||
| return $(() -> { | ||
| List<String> options = new ArrayList<>(); | ||
| List<String> gc =new ArrayList<>(); |
There was a problem hiding this comment.
@jasonk000
Hi, I cannot see the content after nit:. I guess there is a typo.
There was a problem hiding this comment.
It is the whitespace:
-List<String> gc =new ArrayList<>();
+List<String> gc = new ArrayList<>();
| "SELECT s.@displayName as label, s.position as position, s.limit as limit, s.capacity as " + | ||
| "capacity FROM java.nio.DirectByteBuffer s where s.cleaner = null and s.att = null"; | ||
| static final String ALL_BUFFER_OQL = | ||
| "SELECT s.@displayName as label, s.position as position, s.limit as limit, s.capacity as " + |
There was a problem hiding this comment.
Does this break the existing default behaviour now?
Previously the default was equivalent to JDK_MANAGED_BUFFER_OQL, but below we default to ALL. I'm not sure which is intended.
There was a problem hiding this comment.
The frontend will be synced later by @D-D-H
The existing default behavior is problematic because actual direct buffer(and related off-heap memory) is less than presented, that why we provide these fine-grained options to inspect direct buffer.
| return data; | ||
| AnalysisContext context, String mode) throws SnapshotException { | ||
| DirectByteBufferData data = new DirectByteBufferData(); | ||
| Map<String, Object> queryArg; |
There was a problem hiding this comment.
nit: I'd add final here to avoid potential bug from missing break.
|
|
||
| RefinedResultBuilder builder = | ||
| new RefinedResultBuilder(new SnapshotQueryContext(context.snapshot), table); | ||
| builder.setSortOrder(3, Column.SortDirection.DESC); |
There was a problem hiding this comment.
Is sort order important for summary?
There was a problem hiding this comment.
Yes, we want to sort by retaiend heap
|
|
||
| return $(() -> { | ||
| if(type == Model.Histogram.ItemType.PACKAGE){ | ||
| IResult result = queryByCommand(context, "histogram -groupBy " + "BY_PACKAGE"); |
| @@ -71,10 +73,13 @@ PageView<ClassLoader.Item> getChildrenOfClassLoader(int classLoaderId, | |||
| PageView<UnreachableObject.Item> getUnreachableObjects(int page, int pageSize); | |||
|
|
|||
| @ApiMeta(aliases = "directByteBuffer.summary") | |||
| DirectByteBuffer.Summary getSummaryOfDirectByteBuffers(); | |||
| DirectByteBuffer.Summary getSummaryOfDirectByteBuffers(String mode); | |||
There was a problem hiding this comment.
How about making 'mode' optional?
There was a problem hiding this comment.
See queryDirectByteBufferData, default to query all byte buffers.
There was a problem hiding this comment.
optional here means that front-end doesn't have to pass the parameter.
see @ApiParameterMeta.
There was a problem hiding this comment.
How about making mode as an Enum type like 'grouping'?
| queryGc.put("queryString", "SELECT toString(g.name) AS GC FROM sun.management.GarbageCollectorImpl g"); | ||
| return $(() -> { | ||
| List<String> options = new ArrayList<>(); | ||
| List<String> gc =new ArrayList<>(); |
There was a problem hiding this comment.
@jasonk000
Hi, I cannot see the content after nit:. I guess there is a typo.
y1yang0
force-pushed
the
heapdump_more
branch
from
ec02448 to
781b3c7
Compare
Add more backend capabilities for heapdump analysis