Porting tern analysis of JS code to Node. #11948

swmitra · 2015-11-23T09:15:46Z

This PR is created to port our js code analysis logic on node for better scalability . This will improve the overall performance and leverage addition of new features like -

In place rename of variable with scoping [ Separate PR will enable this soon ]
Find reference with scoping [ Separate PR will enable this soon ]
Call Hierarchy [ Separate PR will enable this soon ]

and other related features.

TODO

Fetching of doc content from node domain instead of posting request back to main thread
Unit test update

swmitra · 2015-11-23T09:17:43Z

Tagging @nethip @abose @zaggino @ryanstewart @petetnt

abose · 2015-11-24T08:35:49Z

.gitmodules

@@ -10,6 +10,9 @@
 [submodule "src/extensions/default/JavaScriptCodeHints/thirdparty/tern"]
 	path = src/extensions/default/JavaScriptCodeHints/thirdparty/tern
 	url = https://github.com/marijnh/tern.git
+[submodule "src/extensions/default/JavaScriptCodeHints/node/node_modules/tern"]


two tern submodules. Can this be unified?

swmitra · 2015-11-24T09:21:44Z

@abose WIP! Removed the old reference now.

abose · 2015-11-24T09:23:27Z

Would the path change affect the extensions?

gitmodule mapping changes.

petetnt · 2015-11-30T07:30:30Z

@swmitra Have you done any initial performance benchmarks against the previous implementation yet 📊?

I can try and review this ASAP, been a bit busy as of late

ficristo · 2015-11-30T11:17:26Z

.gitmodules

@@ -25,3 +22,9 @@
 [submodule "src/extensions/default/JSLint/thirdparty/jslint"]
 	path = src/extensions/default/JSLint/thirdparty/jslint
 	url = https://github.com/peterflynn/JSLint.git
+[submodule "src/extensions/default/JavaScriptCodeHints/node/node_modules/acorn"]


Ooc, would not be better to create a package.json in src/extensions/default/JavaScriptCodeHints/node?

@swmitra
we don't need submodules anymore, npm install will install dependencies defined in src/extensions/default/JavaScriptCodeHints/package.json, please remove

ficristo · 2015-11-30T16:46:56Z

@swmitra Do you think it would be possible / easier to add the reading of a .tern-config file (even in a future PR)?

swmitra · 2015-12-01T07:53:47Z

Sorry for late reply.

@petetnt Yes I tried to benchmark the performance to have the gain quantified (Will post a detailed result once the implementation is complete).
Any help on performance data with and without this PR is welcome!
For small and medium JS files , there is marginal gain which may not be even perceptible. The benefits of porting this analysis to tern node has 2 main agendas -

File content extraction can be done from node domain itself. I am currently working on this. This will allow us to perform the complete analysis and preparation for the analysis outside main thread as we don't have to post back messages to the main thread asking for file content.
We do have a bottleneck in terms of performance when we are asking for hints in htmlmixed content and the html file is large enough ( may be 10k+ lines of code ). The typing response gets severely affected as the main thread gets hogged to extract JS text from the html file. I am trying to use a CM runmode on node to offload this to node domain. This will improve the typing performance of the main editor significantly.

@ficristo Yes , it's a good idea to have a package.json and resolve the dependencies as node modules. Also we can introduce a tern config file. I will take both this activities outside this PR and do the changes under a separate PR soon after this.

petetnt · 2015-12-11T08:14:50Z

src/extensions/default/JavaScriptCodeHints/node/ExtractFileContent.js

+
+/*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, maxerr: 50, node: true, regexp: true */
+
+(function () {


The methods in this function need JSDocs I think 📝

@petetnt Sorry for late reply. Was busy with some the type details stuff 😃 . I will add required JSDocs tonight.

Adding JSDoc in ExtractFileContent

# Conflicts: # .gitmodules # src/extensions/default/JavaScriptCodeHints/ScopeManager.js

petetnt

LGTM with a notice

petetnt · 2017-06-07T07:35:54Z

src/extensions/default/JavaScriptCodeHints/node/package.json

+  "name": "brackets-javascript-code-hints",
+  "dependencies": {
+    "acorn": "3.3.0",
+    "tern": "0.20.0"


Latest tern is now 0.21.0. If we can update to that, great. If it has breaking changes, lets keep this one and update later.

No time for another review here.

swmitra · 2017-06-19T13:21:35Z

Pushing this to next release. We will merge this the moment we release 1.10 and do the first pre-release with this PR. It's breaking a few unit tests and have to resolve them before we merge.

marcelgerber · 2017-07-09T12:46:02Z

@swmitra Based on your last comment, this is now ready to be merged.

swmitra · 2017-07-09T13:35:16Z

Yes @marcelgerber 😄 . If we need a build refresh for the reported dll issue, we would be in trouble in case this PR is merged. I would wait for a couple of days to discuss any possibility of build refresh and then merge this PR for the first pre release.

…ra/TernNode

swmitra · 2017-07-12T09:10:16Z

@marcelgerber This PR can't be merged right now 😞. There are 2 major issues I found today while testing -

TOFIX

Require module hints have stopped working
If node restarts for any failure, re initialization of Tern should happen again

While I am working on these 2 issues, if you can spot any problem or a probable cause for this, please let me know. These should be fixed in a day or two.

petetnt · 2017-07-12T09:51:25Z

src/extensions/default/JavaScriptCodeHints/node/ExtractFileContent.js

+ * @param {Function} callback - callback handle to post the content back
+ */
+function _readFile(fileName, callback) {
+    fs.readFile(fileName, "utf8", function (err, data) {


Now that Brackets handles arbitary encodings, should the encoding be passed to this function too?

Precisely I am in a catch 22 situation 😢 . Encoding scheme is only available after detection at shell level, in order to use that, I have to read all the files by hopping on the Brackets main thread, which negates any gain in terms of performance compared to using nodes io. I was relying on Brackets only to fetch dirty files content, but If I start relying on Brackets we will end up with performance issues in case of flat project structure. Prime pump will clog the main thread which in turn will make the entire editor non responsive.

@petetnt I have fixed most of the issues. Relying on Brackets main thread for content until we figure out a way to pass encoding info.

swmitra · 2017-07-12T16:34:53Z

Acorn and Tern have been upgraded to latest version 5.1.1 and 0.21.0 respectively. Haven't noticed any problem in real use or test suite except the ones mentioned before.

swmitra · 2017-07-13T13:55:41Z

@marcelgerber @petetnt Just the final bit remaining. Need to re-initialize Tern when node gets restarted after a crash. Can we merge this PR now? We can get some time for testing all the features then.

petetnt

One more question

petetnt · 2017-07-13T14:21:09Z

src/extensions/default/JavaScriptCodeHints/node/TernNodeDomain.js

+
+require("tern/plugin/requirejs");
+require("tern/plugin/doc_comment");
+require("tern/plugin/angular");


Do we need this?

Yes Pete. Without these, commonjs dependency analysis and js doc parsing for parameter type inference doesn't work.

petetnt · 2017-07-13T14:45:23Z

LGTM with a follow up PR on the restart-after-crash scenario

Porting tern analysis of JS code to Node.

b3ed2b5

swmitra closed this Nov 23, 2015

swmitra reopened this Nov 23, 2015

swmitra added the performance label Nov 23, 2015

swmitra added this to the Release 1.6 milestone Nov 23, 2015

swmitra self-assigned this Nov 23, 2015

abose reviewed Nov 24, 2015
View reviewed changes

Removing old tern submodule config

c8a7df3

swmitra added 4 commits November 26, 2015 17:25

JS text cache in HTML Mixed Mode.

76bbb48

gitmodule mapping changes.

Submodules fix

39088df

Fixing more submodule issues

d35be04

Tern default defs path fix

64f4d75

ficristo reviewed Nov 30, 2015
View reviewed changes

swmitra added 3 commits December 7, 2015 14:27

First draft for content extraction in node [UNIT TESTING IN PROGRESS]

b9d73c1

Update dirty doc content in tern cache.

02420b5

Stability fix.

873431e

petetnt reviewed Dec 11, 2015
View reviewed changes

swmitra added 3 commits December 14, 2015 11:56

Update submodule repo link.

ca7d68d

Adding JSDoc in ExtractFileContent

Merge branch 'master' into swmitra/TernNode

d959165

# Conflicts: # .gitmodules # src/extensions/default/JavaScriptCodeHints/ScopeManager.js

Submodule fix

f726a28

petetnt mentioned this pull request Dec 15, 2015

Unifying third-party dependency management (through npm) #12006

Open

abose added this to the Release 1.7 milestone Feb 24, 2016

zaggino approved these changes Jun 7, 2017

View reviewed changes

petetnt approved these changes Jun 7, 2017

View reviewed changes

petetnt mentioned this pull request Jun 13, 2017

Quick Open doesn't auto-select the first item any more #13437

Closed

3 tasks

swmitra modified the milestones: Release 2.0, Release 1.10 Jun 19, 2017

swmitra modified the milestones: Release 1.10, Release 2.0 Jun 19, 2017

swmitra added 2 commits July 10, 2017 14:42

Merge branch 'master' of https://github.com/adobe/brackets into swmit…

80b762c

…ra/TernNode

Remove tern web worker

34c99ee

petetnt reviewed Jul 12, 2017

View reviewed changes

Update Acorn and Tern to latest available version

2a3cfcd

Bug fixes - Handle require plugin, untitled doc, doc comment plugin

0278ae6

petetnt reviewed Jul 13, 2017

View reviewed changes

petetnt merged commit 1edb0d6 into master Jul 13, 2017

petetnt deleted the swmitra/TernNode branch July 13, 2017 14:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Porting tern analysis of JS code to Node. #11948

Porting tern analysis of JS code to Node. #11948

swmitra commented Nov 23, 2015 •

edited

Loading

swmitra commented Nov 23, 2015

abose Nov 24, 2015

swmitra commented Nov 24, 2015

abose commented Nov 24, 2015

petetnt commented Nov 30, 2015

ficristo Nov 30, 2015

zaggino Jan 18, 2017

ficristo commented Nov 30, 2015

swmitra commented Dec 1, 2015

petetnt Dec 11, 2015

swmitra Dec 11, 2015

petetnt left a comment

petetnt Jun 7, 2017

swmitra commented Jun 19, 2017

marcelgerber commented Jul 9, 2017

swmitra commented Jul 9, 2017 •

edited

Loading

swmitra commented Jul 12, 2017 •

edited

Loading

petetnt Jul 12, 2017

swmitra Jul 12, 2017 •

edited

Loading

swmitra Jul 13, 2017

swmitra commented Jul 12, 2017 •

edited

Loading

swmitra commented Jul 13, 2017 •

edited

Loading

petetnt left a comment

petetnt Jul 13, 2017

swmitra Jul 13, 2017

petetnt commented Jul 13, 2017


		/jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, maxerr: 50, node: true, regexp: true /

		(function () {

Porting tern analysis of JS code to Node. #11948

Porting tern analysis of JS code to Node. #11948

Conversation

swmitra commented Nov 23, 2015 • edited Loading

and other related features.

TODO

swmitra commented Nov 23, 2015

Choose a reason for hiding this comment

swmitra commented Nov 24, 2015

abose commented Nov 24, 2015

petetnt commented Nov 30, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ficristo commented Nov 30, 2015

swmitra commented Dec 1, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petetnt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swmitra commented Jun 19, 2017

marcelgerber commented Jul 9, 2017

swmitra commented Jul 9, 2017 • edited Loading

swmitra commented Jul 12, 2017 • edited Loading

TOFIX

Choose a reason for hiding this comment

swmitra Jul 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

swmitra commented Jul 12, 2017 • edited Loading

swmitra commented Jul 13, 2017 • edited Loading

petetnt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petetnt commented Jul 13, 2017

swmitra commented Nov 23, 2015 •

edited

Loading

swmitra commented Jul 9, 2017 •

edited

Loading

swmitra commented Jul 12, 2017 •

edited

Loading

swmitra Jul 12, 2017 •

edited

Loading

swmitra commented Jul 12, 2017 •

edited

Loading

swmitra commented Jul 13, 2017 •

edited

Loading