From 0b86fa358bf0003aecb0be529dbc3d8eefdbbf94 Mon Sep 17 00:00:00 2001 From: Anatoliy Bazko Date: Tue, 24 Jul 2018 06:19:36 +0000 Subject: [PATCH] Improve java debug performance to fetch list of threads/frames/variables (#10492) * Improve debug performance Signed-off-by: Anatoliy Bazko --- .../ide/api/debug/DebuggerServiceClient.java | 9 ++++++++ .../api/debug/DebuggerServiceClientImpl.java | 9 ++++++++ .../org/eclipse/che/ide/debug/Debugger.java | 8 +++++++ .../debugger/ide/debug/AbstractDebugger.java | 9 ++++++++ .../debugger/ide/debug/DebuggerPresenter.java | 13 +++++++++++- .../che/plugin/jdb/server/JavaDebugger.java | 11 +++++++--- .../plugin/jdb/server/model/JdbLocation.java | 6 ++++++ .../jdb/server/model/JdbStackFrame.java | 21 +++++++++++++------ .../jdb/server/StackFrameDumpTest1.java | 6 +++--- .../plugin/jdb/server/ThreadDumpTest1.java | 6 +++--- .../che/selenium/debugger/ThreadDumpTest.java | 6 +++--- .../che/api/debugger/server/Debugger.java | 13 ++++++++++++ .../api/debugger/server/DebuggerService.java | 13 ++++++++++++ 13 files changed, 111 insertions(+), 19 deletions(-) diff --git a/ide/che-core-ide-api/src/main/java/org/eclipse/che/ide/api/debug/DebuggerServiceClient.java b/ide/che-core-ide-api/src/main/java/org/eclipse/che/ide/api/debug/DebuggerServiceClient.java index b86465d7b6..5ca37bf8c0 100644 --- a/ide/che-core-ide-api/src/main/java/org/eclipse/che/ide/api/debug/DebuggerServiceClient.java +++ b/ide/che-core-ide-api/src/main/java/org/eclipse/che/ide/api/debug/DebuggerServiceClient.java @@ -181,4 +181,13 @@ public interface DebuggerServiceClient { * @param frameIndex the frame index inside the thread */ Promise evaluate(String id, String expression, long threadId, int frameIndex); + + /** + * Gets a location of the resources for the given frame. + * + * @param id debug session id + * @param threadId the unique thread id {@link ThreadState#getId()} + * @param frameIndex the frame index inside the thread + */ + Promise getStackFrameLocation(String id, long threadId, int frameIndex); } diff --git a/ide/che-core-ide-api/src/main/java/org/eclipse/che/ide/api/debug/DebuggerServiceClientImpl.java b/ide/che-core-ide-api/src/main/java/org/eclipse/che/ide/api/debug/DebuggerServiceClientImpl.java index 9f79f2a163..ada16098d4 100644 --- a/ide/che-core-ide-api/src/main/java/org/eclipse/che/ide/api/debug/DebuggerServiceClientImpl.java +++ b/ide/che-core-ide-api/src/main/java/org/eclipse/che/ide/api/debug/DebuggerServiceClientImpl.java @@ -208,6 +208,15 @@ public class DebuggerServiceClientImpl implements DebuggerServiceClient { .send(new StringUnmarshaller()); } + @Override + public Promise getStackFrameLocation(String id, long threadId, int frameIndex) { + String requestUrl = getBaseUrl(id) + "/location?thread=" + threadId + "&frame=" + frameIndex; + return asyncRequestFactory + .createGetRequest(requestUrl) + .loader(loaderFactory.newLoader()) + .send(dtoUnmarshallerFactory.newUnmarshaller(LocationDto.class)); + } + private String getBaseUrl(String id) { final String url = appContext.getWsAgentServerApiEndpoint() + "/debugger"; if (id != null) { diff --git a/ide/che-core-ide-app/src/main/java/org/eclipse/che/ide/debug/Debugger.java b/ide/che-core-ide-app/src/main/java/org/eclipse/che/ide/debug/Debugger.java index 05a3db37e0..4e54f6d5ee 100644 --- a/ide/che-core-ide-app/src/main/java/org/eclipse/che/ide/debug/Debugger.java +++ b/ide/che-core-ide-app/src/main/java/org/eclipse/che/ide/debug/Debugger.java @@ -125,6 +125,14 @@ public interface Debugger extends DebuggerObservable { */ void setValue(Variable variable, long threadId, int frameIndex); + /** + * Gets a location of the resources for the given frame. + * + * @param threadId the unique thread id {@link ThreadState#getId()} + * @param frameIndex the frame index inside the thread + */ + Promise getStackFrameLocation(long threadId, int frameIndex); + /** Indicates if connection is established with the server. */ boolean isConnected(); diff --git a/plugins/plugin-debugger/che-plugin-debugger-ide/src/main/java/org/eclipse/che/plugin/debugger/ide/debug/AbstractDebugger.java b/plugins/plugin-debugger/che-plugin-debugger-ide/src/main/java/org/eclipse/che/plugin/debugger/ide/debug/AbstractDebugger.java index 3c417610fc..a0e044ee54 100644 --- a/plugins/plugin-debugger/che-plugin-debugger-ide/src/main/java/org/eclipse/che/plugin/debugger/ide/debug/AbstractDebugger.java +++ b/plugins/plugin-debugger/che-plugin-debugger-ide/src/main/java/org/eclipse/che/plugin/debugger/ide/debug/AbstractDebugger.java @@ -709,6 +709,15 @@ public abstract class AbstractDebugger implements Debugger, DebuggerObservable { } } + @Override + public Promise getStackFrameLocation(long threadId, int frameIndex) { + if (isConnected()) { + return service.getStackFrameLocation(debugSessionDto.getId(), threadId, frameIndex); + } + + return promiseProvider.reject(JsPromiseError.create("Debugger is not connected")); + } + @Override public boolean isConnected() { return debugSessionDto != null; diff --git a/plugins/plugin-debugger/che-plugin-debugger-ide/src/main/java/org/eclipse/che/plugin/debugger/ide/debug/DebuggerPresenter.java b/plugins/plugin-debugger/che-plugin-debugger-ide/src/main/java/org/eclipse/che/plugin/debugger/ide/debug/DebuggerPresenter.java index df20748e06..33301dd4a0 100644 --- a/plugins/plugin-debugger/che-plugin-debugger-ide/src/main/java/org/eclipse/che/plugin/debugger/ide/debug/DebuggerPresenter.java +++ b/plugins/plugin-debugger/che-plugin-debugger-ide/src/main/java/org/eclipse/che/plugin/debugger/ide/debug/DebuggerPresenter.java @@ -214,7 +214,18 @@ public class DebuggerPresenter extends BasePresenter if (threadState != null) { List frames = threadState.getFrames(); if (frames.size() > frameIndex) { - open(frames.get(frameIndex).getLocation()); + + Debugger debugger = debuggerManager.getActiveDebugger(); + if (debugger != null && debugger.isSuspended()) { + debugger + .getStackFrameLocation(threadId, frameIndex) + .then( + location -> { + if (isSameSelection(threadId, frameIndex)) { + open(location); + } + }); + } } } } diff --git a/plugins/plugin-java-debugger/che-plugin-java-debugger-server/src/main/java/org/eclipse/che/plugin/jdb/server/JavaDebugger.java b/plugins/plugin-java-debugger/che-plugin-java-debugger-server/src/main/java/org/eclipse/che/plugin/jdb/server/JavaDebugger.java index 89ab83a76c..71779817e7 100644 --- a/plugins/plugin-java-debugger/che-plugin-java-debugger-server/src/main/java/org/eclipse/che/plugin/jdb/server/JavaDebugger.java +++ b/plugins/plugin-java-debugger/che-plugin-java-debugger-server/src/main/java/org/eclipse/che/plugin/jdb/server/JavaDebugger.java @@ -76,7 +76,6 @@ import org.eclipse.che.plugin.jdb.server.expression.Evaluator; import org.eclipse.che.plugin.jdb.server.expression.ExpressionException; import org.eclipse.che.plugin.jdb.server.expression.ExpressionParser; import org.eclipse.che.plugin.jdb.server.model.JdbLocation; -import org.eclipse.che.plugin.jdb.server.model.JdbMethod; import org.eclipse.che.plugin.jdb.server.model.JdbStackFrame; import org.eclipse.che.plugin.jdb.server.utils.JavaDebuggerUtils; import org.slf4j.Logger; @@ -405,8 +404,7 @@ public class JavaDebugger implements EventsHandler, Debugger { List frames = new LinkedList<>(); try { for (StackFrame f : t.frames()) { - frames.add( - new JdbStackFrame(f, emptyList(), emptyList(), new JdbLocation(f, new JdbMethod(f)))); + frames.add(new JdbStackFrame(f, emptyList(), emptyList())); } } catch (IncompatibleThreadStateException ignored) { // Thread isn't suspended. Information isn't available. @@ -424,6 +422,7 @@ public class JavaDebugger implements EventsHandler, Debugger { return threadStates; } + /** * Get value of variable with specified path. Each item in path is name of variable. * @@ -722,6 +721,12 @@ public class JavaDebugger implements EventsHandler, Debugger { return result == null ? "null" : result.toString(); } + @Override + public Location getStackFrameLocation(long threadId, int frameIndex) throws DebuggerException { + StackFrame jdiStackFrame = getJdiStackFrame(threadId, frameIndex); + return new JdbLocation(jdiStackFrame); + } + private com.sun.jdi.Value evaluate(ExpressionParser parser, long threadId, int frameIndex) throws DebuggerException { StackFrame jdiStackFrame = getJdiStackFrame(threadId, frameIndex); diff --git a/plugins/plugin-java-debugger/che-plugin-java-debugger-server/src/main/java/org/eclipse/che/plugin/jdb/server/model/JdbLocation.java b/plugins/plugin-java-debugger/che-plugin-java-debugger-server/src/main/java/org/eclipse/che/plugin/jdb/server/model/JdbLocation.java index 72d6403c45..5cf35954e6 100644 --- a/plugins/plugin-java-debugger/che-plugin-java-debugger-server/src/main/java/org/eclipse/che/plugin/jdb/server/model/JdbLocation.java +++ b/plugins/plugin-java-debugger/che-plugin-java-debugger-server/src/main/java/org/eclipse/che/plugin/jdb/server/model/JdbLocation.java @@ -33,6 +33,12 @@ public class JdbLocation implements Location { this(stackFrame, new JdbMethod(stackFrame)); } + public JdbLocation(Location internal, Method method) { + this.method = method; + this.internal = internal; + this.jdiStackFrame = null; + } + /** * Intends to create location when thread is not suspended. Information concerning thread and * frame are not available. diff --git a/plugins/plugin-java-debugger/che-plugin-java-debugger-server/src/main/java/org/eclipse/che/plugin/jdb/server/model/JdbStackFrame.java b/plugins/plugin-java-debugger/che-plugin-java-debugger-server/src/main/java/org/eclipse/che/plugin/jdb/server/model/JdbStackFrame.java index 6b65e3a38c..2d847c042e 100644 --- a/plugins/plugin-java-debugger/che-plugin-java-debugger-server/src/main/java/org/eclipse/che/plugin/jdb/server/model/JdbStackFrame.java +++ b/plugins/plugin-java-debugger/che-plugin-java-debugger-server/src/main/java/org/eclipse/che/plugin/jdb/server/model/JdbStackFrame.java @@ -20,6 +20,7 @@ import org.eclipse.che.api.debug.shared.model.Field; import org.eclipse.che.api.debug.shared.model.Location; import org.eclipse.che.api.debug.shared.model.StackFrameDump; import org.eclipse.che.api.debug.shared.model.Variable; +import org.eclipse.che.api.debug.shared.model.impl.LocationImpl; import org.eclipse.che.api.debug.shared.model.impl.VariablePathImpl; /** @@ -38,20 +39,28 @@ public class JdbStackFrame implements StackFrameDump { public JdbStackFrame(com.sun.jdi.StackFrame jdiStackFrame) { this.jdiStackFrame = jdiStackFrame; - this.location = new JdbLocation(jdiStackFrame); + + com.sun.jdi.Location jdiLocation = jdiStackFrame.location(); + this.location = + new JdbLocation( + new LocationImpl(jdiLocation.declaringType().name(), jdiLocation.lineNumber()), + new JdbMethod(jdiStackFrame)); + this.variables = new AtomicReference<>(); this.fields = new AtomicReference<>(); } public JdbStackFrame( - com.sun.jdi.StackFrame jdiStackFrame, - List fields, - List variables, - Location location) { + com.sun.jdi.StackFrame jdiStackFrame, List fields, List variables) { this.jdiStackFrame = jdiStackFrame; this.fields = new AtomicReference<>(fields); this.variables = new AtomicReference<>(variables); - this.location = location; + + com.sun.jdi.Location jdiLocation = jdiStackFrame.location(); + this.location = + new JdbLocation( + new LocationImpl(jdiLocation.declaringType().name(), jdiLocation.lineNumber()), + new JdbMethod(jdiStackFrame)); } @Override diff --git a/plugins/plugin-java-debugger/che-plugin-java-debugger-server/src/test/java/org/eclipse/che/plugin/jdb/server/StackFrameDumpTest1.java b/plugins/plugin-java-debugger/che-plugin-java-debugger-server/src/test/java/org/eclipse/che/plugin/jdb/server/StackFrameDumpTest1.java index 901ff4887f..f2181976f2 100644 --- a/plugins/plugin-java-debugger/che-plugin-java-debugger-server/src/test/java/org/eclipse/che/plugin/jdb/server/StackFrameDumpTest1.java +++ b/plugins/plugin-java-debugger/che-plugin-java-debugger-server/src/test/java/org/eclipse/che/plugin/jdb/server/StackFrameDumpTest1.java @@ -88,7 +88,7 @@ public class StackFrameDumpTest1 { LocationDto location = stackFrame.getLocation(); assertEquals(location.getLineNumber(), 25); - assertEquals(location.getTarget(), "/test/src/org/eclipse/StackFrameDumpTest1.java"); + assertEquals(location.getTarget(), "org.eclipse.StackFrameDumpTest1"); MethodDto method = location.getMethod(); assertEquals(method.getName(), "do2"); @@ -134,7 +134,7 @@ public class StackFrameDumpTest1 { LocationDto location = stackFrame.getLocation(); assertEquals(location.getLineNumber(), 21); - assertEquals(location.getTarget(), "/test/src/org/eclipse/StackFrameDumpTest1.java"); + assertEquals(location.getTarget(), "org.eclipse.StackFrameDumpTest1"); MethodDto method = location.getMethod(); assertEquals(method.getName(), "do1"); @@ -188,7 +188,7 @@ public class StackFrameDumpTest1 { LocationDto location = stackFrame.getLocation(); assertEquals(location.getLineNumber(), 16); - assertEquals(location.getTarget(), "/test/src/org/eclipse/StackFrameDumpTest1.java"); + assertEquals(location.getTarget(), "org.eclipse.StackFrameDumpTest1"); MethodDto method = location.getMethod(); assertEquals(method.getName(), "main"); diff --git a/plugins/plugin-java-debugger/che-plugin-java-debugger-server/src/test/java/org/eclipse/che/plugin/jdb/server/ThreadDumpTest1.java b/plugins/plugin-java-debugger/che-plugin-java-debugger-server/src/test/java/org/eclipse/che/plugin/jdb/server/ThreadDumpTest1.java index 82c17673cd..778711dbbe 100644 --- a/plugins/plugin-java-debugger/che-plugin-java-debugger-server/src/test/java/org/eclipse/che/plugin/jdb/server/ThreadDumpTest1.java +++ b/plugins/plugin-java-debugger/che-plugin-java-debugger-server/src/test/java/org/eclipse/che/plugin/jdb/server/ThreadDumpTest1.java @@ -100,9 +100,9 @@ public class ThreadDumpTest1 { Location location = stackFrameDump.getLocation(); assertEquals(location.getLineNumber(), 26); - assertEquals(location.getTarget(), "/test/src/org/eclipse/ThreadDumpTest1.java"); - assertEquals(location.getExternalResourceId(), -1); - assertEquals(location.getResourceProjectPath(), "/test"); + assertEquals(location.getTarget(), "org.eclipse.ThreadDumpTest1"); + assertEquals(location.getExternalResourceId(), 0); + assertNull(location.getResourceProjectPath()); Method method = location.getMethod(); assertEquals(method.getName(), "main"); diff --git a/selenium/che-selenium-test/src/test/java/org/eclipse/che/selenium/debugger/ThreadDumpTest.java b/selenium/che-selenium-test/src/test/java/org/eclipse/che/selenium/debugger/ThreadDumpTest.java index 4b0d9012e3..9977b0ef07 100644 --- a/selenium/che-selenium-test/src/test/java/org/eclipse/che/selenium/debugger/ThreadDumpTest.java +++ b/selenium/che-selenium-test/src/test/java/org/eclipse/che/selenium/debugger/ThreadDumpTest.java @@ -97,15 +97,15 @@ public class ThreadDumpTest { String[] frames = debugPanel.getFrames(); assertEquals(frames.length, 1); - assertTrue(frames[0].contains("main(String[]):19, App")); + assertTrue(frames[0].contains("main(String[]):19, multimodule.App")); debugPanel.clickOnButton(DebugPanel.DebuggerActionButtons.RESUME_BTN_ID); debugPanel.waitDebugHighlightedText("this.title = title;"); frames = debugPanel.getFrames(); assertEquals(frames.length, 2); - assertTrue(frames[0].contains("(String, String):18, BookImpl")); - assertTrue(frames[1].contains("main(String[]):19, App")); + assertTrue(frames[0].contains("(String, String):18, multimodule.model.BookImpl")); + assertTrue(frames[1].contains("main(String[]):19, multimodule.App")); editor.closeAllTabs(); diff --git a/wsagent/che-core-api-debug/src/main/java/org/eclipse/che/api/debugger/server/Debugger.java b/wsagent/che-core-api-debug/src/main/java/org/eclipse/che/api/debugger/server/Debugger.java index 6a390942cf..b386550c8a 100644 --- a/wsagent/che-core-api-debug/src/main/java/org/eclipse/che/api/debugger/server/Debugger.java +++ b/wsagent/che-core-api-debug/src/main/java/org/eclipse/che/api/debugger/server/Debugger.java @@ -244,6 +244,19 @@ public interface Debugger { return Collections.emptyList(); } + /** + * Gets a location of the resources for the given frame. + * + * @throws DebuggerException if any error occur + */ + default Location getStackFrameLocation(long threadId, int frameIndex) throws DebuggerException { + if (threadId == -1) { + return dumpStackFrame().getLocation(); + } + + return getStackFrameDump(threadId, frameIndex).getLocation(); + } + /** Is used to send back any events to client. */ interface DebuggerCallback { void onEvent(DebuggerEvent event); diff --git a/wsagent/che-core-api-debug/src/main/java/org/eclipse/che/api/debugger/server/DebuggerService.java b/wsagent/che-core-api-debug/src/main/java/org/eclipse/che/api/debugger/server/DebuggerService.java index 3d1bdff715..6f7014d34f 100644 --- a/wsagent/che-core-api-debug/src/main/java/org/eclipse/che/api/debugger/server/DebuggerService.java +++ b/wsagent/che-core-api-debug/src/main/java/org/eclipse/che/api/debugger/server/DebuggerService.java @@ -34,6 +34,7 @@ import javax.ws.rs.core.MultivaluedMap; import javax.ws.rs.core.UriInfo; import org.eclipse.che.api.debug.shared.dto.BreakpointDto; import org.eclipse.che.api.debug.shared.dto.DebugSessionDto; +import org.eclipse.che.api.debug.shared.dto.LocationDto; import org.eclipse.che.api.debug.shared.dto.SimpleValueDto; import org.eclipse.che.api.debug.shared.dto.StackFrameDumpDto; import org.eclipse.che.api.debug.shared.dto.ThreadStateDto; @@ -197,6 +198,18 @@ public class DebuggerService { return threadStates.stream().map(DtoConverter::asDto).collect(Collectors.toList()); } + @GET + @Path("{id}/location") + @Produces(MediaType.APPLICATION_JSON) + public LocationDto getStackFrameLocation( + @PathParam("id") String sessionId, + @QueryParam("thread") @DefaultValue("-1") long threadId, + @QueryParam("frame") @DefaultValue("-1") int frameIndex) + throws DebuggerException { + return DtoConverter.asDto( + debuggerManager.getDebugger(sessionId).getStackFrameLocation(threadId, frameIndex)); + } + @GET @Path("{id}/value") @Produces(MediaType.APPLICATION_JSON)