[ ZEPPELIN-1266 ] Code editor Optimization by cloverhearts · Pull Request #1258 · apache/zeppelin

cloverhearts · 2016-08-02T09:06:38Z

What is this PR for?

If the result of many para- graph, the code editor is very slow.

What type of PR is it?

Improvement

Todos

removed call to function in template.
changed event (aceChange event to ace input event)

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-1266

How should this be tested?

going to r tutorial notebook.
write to paragraph on anyway. (many many.. fast.. fast typing.)

Screenshots (if appropriate)

Before optimization.

After optimization.

Questions:

Does the licenses files need update? no
Is there breaking changes for older versions? no
Does this needs documentation? no

cloverhearts · 2016-08-02T09:07:28Z

ci trigger.

cloverhearts · 2016-08-02T09:27:33Z

please, remove to browser cache

bzz · 2016-08-02T09:37:47Z

zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js

+      $scope.isParagraphSaveAlready = true;
+      $scope.startSaveTimer();
+      var cursorValue = $scope.editor.getCursorPosition();
+      if (cursorValue.row === 0 && cursorValue <= 30) {


AFAIK here, in case it's the first row (like %something.somethig-else) we change paragraph mode.
But why do we check cursorValue <= 30 ?

bzz · 2016-08-02T09:38:50Z

Would it make sense to change isParagraphSaveAlready => wasParagraphAlreadySaved ?

cloverhearts · 2016-08-02T10:01:04Z

@bzz Thank you for your feedback.
i will check to your opinion.
Thank you

corneadoug · 2016-08-02T12:07:40Z

I would limit this PR to the first commit, for me the rest is out of scope.
Also, in the first PR, there is no need to make a if before calling setParagraphMode, the conditions you added are already at the beginning of the function.

$scope.setParagraphMode can also be changed to var setParagraphMode

cloverhearts · 2016-08-03T05:07:07Z

@corneadoug
I did modification.
Please review for this pr.
Thank you.

corneadoug · 2016-08-03T05:07:24Z

zeppelin-web/src/app/notebook/paragraph/paragraph.controller.js

-    $timeout(function() {
-      $scope.setParagraphMode($scope.editor.getSession(), $scope.dirtyText, $scope.editor.getCursorPosition());
-    });
+    var setParagraphMode = $scope.setParagraphMode;


Why not calling directly the function here?

@corneadoug
I seem to have misunderstood your say.
Modification was completed.
Thank you.

cloverhearts · 2016-08-03T05:07:51Z

@bzz
I removed the parts which you say.
Thank you.

…ditor optimization.

bzz · 2016-08-03T05:47:32Z

Looks great to me!

corneadoug · 2016-08-03T06:00:49Z

@cloverhearts Thanks for the changes and keeping this PR focused.
I saw some great improvements with big Notebook with those changes.
@Leemoonsoo Can you try it too and tell us if you also think its better?

minahlee · 2016-08-05T01:34:25Z

Merge if there is no further discussion

### What is this PR for? If the result of many para- graph, the code editor is very slow. ### What type of PR is it? Improvement ### Todos - [x] removed call to function in template. - [x] changed event (aceChange event to ace input event) ### What is the Jira issue? https://issues.apache.org/jira/browse/ZEPPELIN-1266 ### How should this be tested? 1. going to r tutorial notebook. 2. write to paragraph on anyway. (many many.. fast.. fast typing.) ### Screenshots (if appropriate) #### Before optimization. ![performance02](https://cloud.githubusercontent.com/assets/10525473/17323111/a1cd9b2e-58db-11e6-8d61-7ab98ea96b3a.gif) #### After optimization. ![performance01](https://cloud.githubusercontent.com/assets/10525473/17323107/a02b5338-58db-11e6-95c8-543aab7131dd.gif) ### Questions: * Does the licenses files need update? no * Is there breaking changes for older versions? no * Does this needs documentation? no Author: CloverHearts <cloverheartsdev@gmail.com> Closes #1258 from cloverhearts/dev/aceeditorperformance and squashes the following commits: 2bde378 [CloverHearts] removed local variable setParagarphmode to global variable for code editor optimization. 9e2c7fc [CloverHearts] restore to aceChanged function for code editor optimization 86ba5c4 [CloverHearts] ace editor performance up (cherry picked from commit adf3355) Signed-off-by: Mina Lee <minalee@apache.org>

### What is this PR for? If the result of many para- graph, the code editor is very slow. ### What type of PR is it? Improvement ### Todos - [x] removed call to function in template. - [x] changed event (aceChange event to ace input event) ### What is the Jira issue? https://issues.apache.org/jira/browse/ZEPPELIN-1266 ### How should this be tested? 1. going to r tutorial notebook. 2. write to paragraph on anyway. (many many.. fast.. fast typing.) ### Screenshots (if appropriate) #### Before optimization. ![performance02](https://cloud.githubusercontent.com/assets/10525473/17323111/a1cd9b2e-58db-11e6-8d61-7ab98ea96b3a.gif) #### After optimization. ![performance01](https://cloud.githubusercontent.com/assets/10525473/17323107/a02b5338-58db-11e6-95c8-543aab7131dd.gif) ### Questions: * Does the licenses files need update? no * Is there breaking changes for older versions? no * Does this needs documentation? no Author: CloverHearts <cloverheartsdev@gmail.com> Closes apache#1258 from cloverhearts/dev/aceeditorperformance and squashes the following commits: 2bde378 [CloverHearts] removed local variable setParagarphmode to global variable for code editor optimization. 9e2c7fc [CloverHearts] restore to aceChanged function for code editor optimization 86ba5c4 [CloverHearts] ace editor performance up

ace editor performance up

86ba5c4

cloverhearts closed this Aug 2, 2016

cloverhearts reopened this Aug 2, 2016

bzz reviewed Aug 2, 2016
View reviewed changes

cloverhearts changed the title ~~[ ZEPPELIN-1266 ] Code editor Optimization~~ [ ZEPPELIN-1266 ] Code editor Optimization and Paragraph render logic Optimization Aug 2, 2016

restore to aceChanged function for code editor optimization

9e2c7fc

cloverhearts changed the title ~~[ ZEPPELIN-1266 ] Code editor Optimization and Paragraph render logic Optimization~~ [ ZEPPELIN-1266 ] Code editor Optimization Aug 3, 2016

corneadoug reviewed Aug 3, 2016
View reviewed changes

removed local variable setParagarphmode to global variable for code e…

2bde378

…ditor optimization.

asfgit closed this in adf3355 Aug 5, 2016

Conversation

cloverhearts commented Aug 2, 2016

What is this PR for?

What type of PR is it?

Todos

What is the Jira issue?

How should this be tested?

Screenshots (if appropriate)

Before optimization.

After optimization.

Questions:

Uh oh!

cloverhearts commented Aug 2, 2016

Uh oh!

cloverhearts commented Aug 2, 2016

Uh oh!

bzz Aug 2, 2016

Choose a reason for hiding this comment

Uh oh!

bzz commented Aug 2, 2016

Uh oh!

cloverhearts commented Aug 2, 2016

Uh oh!

corneadoug commented Aug 2, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cloverhearts commented Aug 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

corneadoug Aug 3, 2016

Choose a reason for hiding this comment

Uh oh!

cloverhearts Aug 3, 2016

Choose a reason for hiding this comment

Uh oh!

cloverhearts commented Aug 3, 2016

Uh oh!

bzz commented Aug 3, 2016

Uh oh!

corneadoug commented Aug 3, 2016

Uh oh!

minahlee commented Aug 5, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

corneadoug commented Aug 2, 2016 •

edited

Loading

cloverhearts commented Aug 3, 2016 •

edited

Loading