Skip to content

Enable async jdwp based on network latency for auto mode #447

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Jinbo Wang (testforstephen) merged 1 commit into from
Sep 28, 2022
Merged

Enable async jdwp based on network latency for auto mode #447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion ...a.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/DebugAdapterContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ public class DebugAdapterContext implements IDebugAdapterContext {
private long processId = -1;

private boolean localDebugging = true;
private long jdwpLatency = 0;

private IdCollection<String> sourceReferences = new IdCollection<>();
private RecyclableObjectPool<Long, Object> recyclableIdPool = new RecyclableObjectPool<>();
Expand Down Expand Up @@ -372,7 +373,21 @@ public ThreadCache getThreadCache() {

@Override
public boolean asyncJDWP() {
return DebugSettings.getCurrent().asyncJDWP == AsyncMode.ON;
/**
* If we take 1 second as the acceptable latency for DAP requests,
* With a single-threaded strategy for handling JDWP requests,
* a latency of about 15ms per JDWP request can ensure the responsiveness
* for most DAPs. It allows sending 66 JDWP requests within 1 seconds,
* which can cover most DAP operations such as breakpoint, threads,
* call stack, step and continue.
*/
return asyncJDWP(15);
}

@Override
public boolean asyncJDWP(long usableLatency) {
return DebugSettings.getCurrent().asyncJDWP == AsyncMode.ON
|| (DebugSettings.getCurrent().asyncJDWP == AsyncMode.AUTO && this.jdwpLatency > usableLatency);
}

public boolean isLocalDebugging() {
Expand All @@ -382,4 +397,14 @@ public boolean isLocalDebugging() {
public void setLocalDebugging(boolean local) {
this.localDebugging = local;
}

@Override
public long getJDWPLatency() {
return this.jdwpLatency;
}

@Override
public void setJDWPLatency(long baseLatency) {
this.jdwpLatency = baseLatency;
}
}
6 changes: 6 additions & 0 deletions ....debug.core/src/main/java/com/microsoft/java/debug/core/adapter/IDebugAdapterContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,13 @@ public interface IDebugAdapterContext {

boolean asyncJDWP();

boolean asyncJDWP(long usableLatency/**ms*/);

boolean isLocalDebugging();

void setLocalDebugging(boolean local);

long getJDWPLatency();

void setJDWPLatency(long baseLatency);
}
6 changes: 4 additions & 2 deletions ...ore/src/main/java/com/microsoft/java/debug/core/adapter/handler/AttachRequestHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,10 @@ public CompletableFuture<Response> handle(Command command, Arguments arguments,
long sent = System.currentTimeMillis();
request.enable();
long received = System.currentTimeMillis();
logger.info("Network latency for JDWP command: " + (received - sent) + "ms");
traceInfo.put("networkLatency", (received - sent));
long latency = received - sent;
context.setJDWPLatency(latency);
logger.info("Network latency for JDWP command: " + latency + "ms");
traceInfo.put("networkLatency", latency);
}

IEvaluationProvider evaluationProvider = context.getProvider(IEvaluationProvider.class);
Expand Down
30 changes: 26 additions & 4 deletions ...e/src/main/java/com/microsoft/java/debug/core/adapter/handler/EvaluateRequestHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2017-2021 Microsoft Corporation and others.
* Copyright (c) 2017-2022 Microsoft Corporation and others.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
Expand Down Expand Up @@ -65,7 +65,9 @@ public CompletableFuture<Response> handle(Command command, Arguments arguments,
String expression = evalArguments.expression;

// Async mode is supposed to be performant, then disable the advanced features like hover evaluation.
if (!context.isLocalDebugging() && context.asyncJDWP() && "hover".equals(evalArguments.context)) {
if (context.asyncJDWP(VariablesRequestHandler.USABLE_JDWP_LATENCY)
&& context.getJDWPLatency() > VariablesRequestHandler.USABLE_JDWP_LATENCY
&& "hover".equals(evalArguments.context)) {
return CompletableFuture.completedFuture(response);
}

Expand Down Expand Up @@ -98,7 +100,7 @@ public CompletableFuture<Response> handle(Command command, Arguments arguments,
Value sizeValue = null;
if (value instanceof ArrayReference) {
indexedVariables = ((ArrayReference) value).length();
} else if (value instanceof ObjectReference && DebugSettings.getCurrent().showLogicalStructure && engine != null) {
} else if (value instanceof ObjectReference && supportsLogicStructureView(context, evalArguments.context) && engine != null) {
try {
JavaLogicalStructure structure = JavaLogicalStructureManager.getLogicalStructure((ObjectReference) value);
if (structure != null && structure.getSizeExpression() != null) {
Expand Down Expand Up @@ -135,7 +137,7 @@ public CompletableFuture<Response> handle(Command command, Arguments arguments,
// If failed to resolve the variable value, skip the details info as well.
} else if (sizeValue != null) {
detailsString = "size=" + variableFormatter.valueToString(sizeValue, options);
} else if (DebugSettings.getCurrent().showToString) {
} else if (supportsToStringView(context, evalArguments.context)) {
try {
detailsString = VariableDetailUtils.formatDetailsValue(value, stackFrameReference.getThread(), variableFormatter, options, engine);
} catch (OutOfMemoryError e) {
Expand Down Expand Up @@ -182,4 +184,24 @@ public CompletableFuture<Response> handle(Command command, Arguments arguments,
}
});
}

private boolean supportsLogicStructureView(IDebugAdapterContext context, String evalContext) {
if (!"watch".equals(evalContext)) {
return true;
}

return (!context.asyncJDWP(VariablesRequestHandler.USABLE_JDWP_LATENCY)
|| context.getJDWPLatency() <= VariablesRequestHandler.USABLE_JDWP_LATENCY)
&& DebugSettings.getCurrent().showLogicalStructure;
}

private boolean supportsToStringView(IDebugAdapterContext context, String evalContext) {
if (!"watch".equals(evalContext)) {
return true;
}

return (!context.asyncJDWP(VariablesRequestHandler.USABLE_JDWP_LATENCY)
|| context.getJDWPLatency() <= VariablesRequestHandler.USABLE_JDWP_LATENCY)
&& DebugSettings.getCurrent().showToString;
}
}
3 changes: 2 additions & 1 deletion ...c/main/java/com/microsoft/java/debug/core/adapter/handler/InlineValuesRequestHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ public CompletableFuture<Response> handle(Command command, Arguments arguments,
}

// Async mode is supposed to be performant, then disable the advanced features like inline values.
if (!context.isLocalDebugging() && context.asyncJDWP()) {
if (context.getJDWPLatency() > VariablesRequestHandler.USABLE_JDWP_LATENCY
&& context.asyncJDWP(VariablesRequestHandler.USABLE_JDWP_LATENCY)) {
response.body = new Responses.InlineValuesResponse(null);
return CompletableFuture.completedFuture(response);
}
Expand Down
31 changes: 26 additions & 5 deletions .../src/main/java/com/microsoft/java/debug/core/adapter/handler/VariablesRequestHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,21 @@

public class VariablesRequestHandler implements IDebugRequestHandler {
protected static final Logger logger = Logger.getLogger(Configuration.LOGGER_NAME);
/**
* When the debugger enables logical structures and
* toString settings, for each Object variable in the
* variable list, the debugger needs to check its
* superclass and interface to find out if it inherits
* from Collection or overrides the toString method.
* This will cause the debugger to send a lot of JDWP
* requests for them. For a test case with 4 object
* variables, the debug adapter may need to send more
* than 100 JDWP requests to handle these variable
* requests. To achieve a DAP latency of 1s with a
* single-threaded JDWP request processing strategy,
* a single JDWP latency is about 10ms.
*/
static final long USABLE_JDWP_LATENCY = 10/**ms*/;

@Override
public List<Command> getTargetCommands() {
Expand Down Expand Up @@ -130,7 +145,7 @@ public CompletableFuture<Response> handle(Command command, Arguments arguments,
childrenList.add(new Variable(returnIcon + result.method.name() + "()", result.value, null));
}

if (context.asyncJDWP()) {
if (useAsyncJDWP(context)) {
childrenList.addAll(getVariablesOfFrameAsync(frame, showStaticVariables));
} else {
childrenList.addAll(VariableUtils.listLocalVariables(frame));
Expand Down Expand Up @@ -198,7 +213,7 @@ public CompletableFuture<Response> handle(Command command, Arguments arguments,
if (varArgs.count > 0) {
childrenList = VariableUtils.listFieldVariables(containerObj, varArgs.start, varArgs.count);
} else {
childrenList = VariableUtils.listFieldVariables(containerObj, showStaticVariables, context.asyncJDWP());
childrenList = VariableUtils.listFieldVariables(containerObj, showStaticVariables, useAsyncJDWP(context));
}
}
} catch (AbsentInformationException e) {
Expand All @@ -215,7 +230,7 @@ public CompletableFuture<Response> handle(Command command, Arguments arguments,
.filter(var -> duplicateNames.contains(var.name))
.collect(Collectors.toList());
// Since JDI caches the fetched properties locally, in async mode we can warm up the JDI cache in advance.
if (context.asyncJDWP()) {
if (useAsyncJDWP(context)) {
try {
AsyncJdwpUtils.await(warmUpJDICache(childrenList, duplicateVars));
} catch (CompletionException | CancellationException e) {
Expand Down Expand Up @@ -377,11 +392,17 @@ public CompletableFuture<Response> handle(Command command, Arguments arguments,
}

private boolean supportsLogicStructureView(IDebugAdapterContext context) {
return (!context.asyncJDWP() || context.isLocalDebugging()) && DebugSettings.getCurrent().showLogicalStructure;
return (!useAsyncJDWP(context) || context.getJDWPLatency() <= USABLE_JDWP_LATENCY)
&& DebugSettings.getCurrent().showLogicalStructure;
}

private boolean supportsToStringView(IDebugAdapterContext context) {
return (!context.asyncJDWP() || context.isLocalDebugging()) && DebugSettings.getCurrent().showToString;
return (!useAsyncJDWP(context) || context.getJDWPLatency() <= USABLE_JDWP_LATENCY)
&& DebugSettings.getCurrent().showToString;
}

private boolean useAsyncJDWP(IDebugAdapterContext context) {
return context.asyncJDWP(USABLE_JDWP_LATENCY);
}

private Types.Variable resolveLazyVariable(IDebugAdapterContext context, VariableProxy containerNode, IVariableFormatter variableFormatter,
Expand Down