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.

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

[PERF] optimize debug lines in the editor #4236

Closed
jasonLaster opened this Issue Oct 2, 2017 · 3 comments

Comments

Projects
None yet
3 participants
@jasonLaster
Contributor

jasonLaster commented Oct 2, 2017

It looks like we're spending a lot of time in setDebugLine and clearDebugLine.

The way I'd debug this is to open sensorWeb w/ the launchpad and add console.log and console.time calls in those two functions and see where we're spending the time.

@jasonLaster jasonLaster changed the title from [PERF] optimize debug lines in the editor to i'[PERF] optimize debug lines in the editor Oct 3, 2017

@hugsyy

This comment has been minimized.

Show comment
Hide comment
@hugsyy

hugsyy Oct 3, 2017

Contributor

Hey, can I work on this?

Contributor

hugsyy commented Oct 3, 2017

Hey, can I work on this?

@jasonLaster

This comment has been minimized.

Show comment
Hide comment
@jasonLaster

jasonLaster Oct 3, 2017

Contributor

@hughugsy sure.

This fixes it, but we should still try and keep the marked text. Perhaps the problem was the setState which would do an endless loop...

diff --git a/src/components/Editor/DebugLine.js b/src/components/Editor/DebugLine.js
index f92c616..7eab45f 100644
--- a/src/components/Editor/DebugLine.js
+++ b/src/components/Editor/DebugLine.js
@@ -17,6 +17,8 @@ export default class DebugLine extends Component {
     }
   };

+  debugExpression: Object;
+
   constructor() {
     super();
     this.state = { debugExpression: { clear: () => {} } };
@@ -57,19 +59,19 @@ export default class DebugLine extends Component {
     }

     doc.addLineClass(line, "line", "new-debug-line");
-    const debugExpression = markText(editor, "debug-expression", {
-      start: { line, column },
-      end: { line, column: null }
-    });
-    this.setState({ debugExpression });
+    // const debugExpression = markText(editor, "debug-expression", {
+    //   start: { line, column },
+    //   end: { line, column: null }
+    // });
+    // this.debugExpression = debugExpression;
   }

   clearDebugLine(selectedFrame: Object, editor: Object) {
     const { line, sourceId } = selectedFrame.location;
-    const { debugExpression } = this.state;
-    if (debugExpression) {
-      debugExpression.clear();
-    }
+    // const { debugExpression } = this;
+    // if (debugExpression) {
+    //   debugExpression.clear();
+    // }

     const editorLine = line - 1;
     const doc = getDocument(sourceId);
Contributor

jasonLaster commented Oct 3, 2017

@hughugsy sure.

This fixes it, but we should still try and keep the marked text. Perhaps the problem was the setState which would do an endless loop...

diff --git a/src/components/Editor/DebugLine.js b/src/components/Editor/DebugLine.js
index f92c616..7eab45f 100644
--- a/src/components/Editor/DebugLine.js
+++ b/src/components/Editor/DebugLine.js
@@ -17,6 +17,8 @@ export default class DebugLine extends Component {
     }
   };

+  debugExpression: Object;
+
   constructor() {
     super();
     this.state = { debugExpression: { clear: () => {} } };
@@ -57,19 +59,19 @@ export default class DebugLine extends Component {
     }

     doc.addLineClass(line, "line", "new-debug-line");
-    const debugExpression = markText(editor, "debug-expression", {
-      start: { line, column },
-      end: { line, column: null }
-    });
-    this.setState({ debugExpression });
+    // const debugExpression = markText(editor, "debug-expression", {
+    //   start: { line, column },
+    //   end: { line, column: null }
+    // });
+    // this.debugExpression = debugExpression;
   }

   clearDebugLine(selectedFrame: Object, editor: Object) {
     const { line, sourceId } = selectedFrame.location;
-    const { debugExpression } = this.state;
-    if (debugExpression) {
-      debugExpression.clear();
-    }
+    // const { debugExpression } = this;
+    // if (debugExpression) {
+    //   debugExpression.clear();
+    // }

     const editorLine = line - 1;
     const doc = getDocument(sourceId);

@jasonLaster jasonLaster added in progress and removed help wanted labels Oct 3, 2017

@jasonLaster jasonLaster changed the title from i'[PERF] optimize debug lines in the editor to [PERF] optimize debug lines in the editor Oct 3, 2017

@sw-yx

This comment has been minimized.

Show comment
Hide comment
@sw-yx

sw-yx Oct 4, 2017

Hi, we tried debugging this today at ReactNYC:

We initially thought the issue was about clicking and unclicking the debug gutter line, but Jason clarified in person that the problem is actually the time it takes to arrive in the editor context relevant to a breakpoint. We were not able to replicate the long delay in a reasonable-sized react app but clarifying the issue here for future people tackling this

sw-yx commented Oct 4, 2017

Hi, we tried debugging this today at ReactNYC:

We initially thought the issue was about clicking and unclicking the debug gutter line, but Jason clarified in person that the problem is actually the time it takes to arrive in the editor context relevant to a breakpoint. We were not able to replicate the long delay in a reasonable-sized react app but clarifying the issue here for future people tackling this

This was referenced Oct 4, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment