DevTools: speed up closure dependency checking

DevTools: speed up closure dependency checking

Compiling frontend runs in ~13 seconds (including dependency check) instead of 22 seconds.

Examples of the RegEx match:
http://regexr.com/3etk5

The following CL needs to land before this CL lands:
https://codereview.chromium.org/2623743002/
BUG=673445
Review-Url: https://codereview.chromium.org/2588843002
Cr-Commit-Position: refs/heads/master@{#442754}
Committed: https://chromium.googlesource.com/chromium/src/+/2bc46a6dd144aa85e0aecd8f16631dc6bbdfa11e

[chenwilliam, 2016-12-19 18:45:14]

Description was changed from

==========
DevTools: create dependency checking script

BUG=
==========

to

==========
DevTools: create dependency checking script

For a file in common module...

INVALID EXAMPLES
----------------
SDK.MakeWhiz
SDK.Target
SDK.Target()
 SDK.Hi
 SDK.W SDK.Foo
SDK.call(SDK.generic)
Common.method(SDK.Events)
Common.method(1, SDK.Events)
SDK[MakeWhiz]
// Hello Console.h
/** @type {SDK.Foo} */
/** @type {!SDK.Foo} */
/** @type {?SDK.Foo} */
/** @type SDK.Foo */
/** @type !SDK.Foo */
/** @type ?SDK.Foo */

VALID EXAMPLES
----------------
// Hello "Console.h"
SDK
Unknown.SDK.hi
Common.SDK.forever
Foo
   .SDK.some
/** @type {Common.SDK.Foo} */
/** @type {!Common.SDK.Foo} */
/** @type {?Common.SDK.Foo} */
/** @type Common.SDK.Foo */
/** @type !Common.SDK.Foo */
/** @type ?Common.SDK.Foo */
var obj = {
 SDK: 1
}

BUG=
==========

[chenwilliam, 2016-12-19 18:45:14]

chenwilliam@chromium.org changed reviewers:
+ dgozman@chromium.org, pfeldman@chromium.org

[chenwilliam, 2016-12-19 18:49:43]

This is a work in progress, but so far this approach looks promising as there's
only a couple of tweaks I had to make to analyze the frontend.

* Not sure how to deal with extensions, so I've skipped it as a module
* I think we can run this concurrently with the closure analysis since this
takes ~6 seconds. We would just need to report dependency violations with line
number and file name.

[chenwilliam, 2016-12-20 02:49:52]

Description was changed from

==========
DevTools: create dependency checking script

For a file in common module...

INVALID EXAMPLES
----------------
SDK.MakeWhiz
SDK.Target
SDK.Target()
 SDK.Hi
 SDK.W SDK.Foo
SDK.call(SDK.generic)
Common.method(SDK.Events)
Common.method(1, SDK.Events)
SDK[MakeWhiz]
// Hello Console.h
/** @type {SDK.Foo} */
/** @type {!SDK.Foo} */
/** @type {?SDK.Foo} */
/** @type SDK.Foo */
/** @type !SDK.Foo */
/** @type ?SDK.Foo */

VALID EXAMPLES
----------------
// Hello "Console.h"
SDK
Unknown.SDK.hi
Common.SDK.forever
Foo
   .SDK.some
/** @type {Common.SDK.Foo} */
/** @type {!Common.SDK.Foo} */
/** @type {?Common.SDK.Foo} */
/** @type Common.SDK.Foo */
/** @type !Common.SDK.Foo */
/** @type ?Common.SDK.Foo */
var obj = {
 SDK: 1
}

BUG=
==========

to

==========
DevTools: speed up closure dependency checking

Compiling frontend runs in ~13 seconds (including dependency check) instead of
22 seconds.

BUG=673445
==========

[chenwilliam, 2016-12-20 02:50:54]

Description was changed from

==========
DevTools: speed up closure dependency checking

Compiling frontend runs in ~13 seconds (including dependency check) instead of
22 seconds.

BUG=673445
==========

to

==========
DevTools: speed up closure dependency checking

Compiling frontend runs in ~13 seconds (including dependency check) instead of
22 seconds.

Examples of the RegEx match:
http://regexr.com/3etk5

BUG=673445
==========

[chenwilliam, 2016-12-20 02:57:46]

PTAL. I think the remaining steps are to rename UI Lazy and Component Lazy
modules to their own namespaces as well as one-off cases like Multimap

[dgozman, 2016-12-20 07:02:18]

https://codereview.chromium.org/2588843002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/scripts/closure/closure_runner/src/org/chromium/devtools/compiler/DevToolsCodingConvention.java
(left):

https://codereview.chromium.org/2588843002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/scripts/closure/closure_runner/src/org/chromium/devtools/compiler/DevToolsCodingConvention.java:14:
return name.length() > 1 && name.charAt(0) == '_' && name.charAt(1) != '_';
What is this replaced with?

https://codereview.chromium.org/2588843002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/scripts/dependency_checker.py (right):

https://codereview.chromium.org/2588843002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/scripts/dependency_checker.py:12:
skip_modules = ['extensions']
why?

https://codereview.chromium.org/2588843002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/scripts/dependency_checker.py:62: if module
== 'ui_lazy':
components_lazy?

[chenwilliam, 2016-12-20 20:23:35]

PTAL.

https://codereview.chromium.org/2588843002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/devtools/scripts/closure/closure_runner/src/org/chromium/devtools/compiler/DevToolsCodingConvention.java
(left):

https://codereview.chromium.org/2588843002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/scripts/closure/closure_runner/src/org/chromium/devtools/compiler/DevToolsCodingConvention.java:14:
return name.length() > 1 && name.charAt(0) == '_' && name.charAt(1) != '_';
On 2016/12/20 at 07:02:18, dgozman wrote:
> What is this replaced with?

I was too eager to delete closure runner :( I did some research but I can't find
a simple way to use our own coding convention. I've restored the closure_runner
but we could remove most of the logic. I'm not very familiar with Java so I'd
like to clean up the Runner.java in a separate CL.

https://codereview.chromium.org/2588843002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/scripts/dependency_checker.py (right):

https://codereview.chromium.org/2588843002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/scripts/dependency_checker.py:12:
skip_modules = ['extensions']
On 2016/12/20 at 07:02:18, dgozman wrote:
> why?

Good catch. I thought it was an issue because it was renaming local variables
with the same name as namespaces but it still compiles so I've removed it.

https://codereview.chromium.org/2588843002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/scripts/dependency_checker.py:62: if module
== 'ui_lazy':
On 2016/12/20 at 07:02:18, dgozman wrote:
> components_lazy?

components_lazy is handled properly by the default logic (to_camel_case).

[chenwilliam, 2016-12-22 01:12:01]

Patchset #5 (id:80001) has been deleted

[chenwilliam, 2016-12-22 18:27:15]

PTAL. I've trimmed down the closure_runner java code to just what we need.

[dgozman, 2016-12-27 18:07:36]

https://codereview.chromium.org/2588843002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/gonzales/module.json (right):

https://codereview.chromium.org/2588843002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/gonzales/module.json:6:
"../common/TextRange.js",
TextRange is a part of formatter_worker, which is a dependency. Why is it
neccessary to mention TextRange here as well?

https://codereview.chromium.org/2588843002/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/scripts/closure/closure_runner/src/org/chromium/devtools/compiler/Runner.java
(right):

https://codereview.chromium.org/2588843002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/scripts/closure/closure_runner/src/org/chromium/devtools/compiler/Runner.java:191:
private static class Flags {
Why do we need all these classes (Flags, CompilerInstanceDescriptor, etc.)? Can
we just pass our coding conventions and let their runner handle command line
flags?

https://codereview.chromium.org/2588843002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/scripts/dependency_checker.py (right):

https://codereview.chromium.org/2588843002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/scripts/dependency_checker.py:16: class
DependencyChecker(object):
AFAIU, this is not a checker per se, but rather a preprocessor.

[chenwilliam, 2016-12-27 23:49:15]

PTAL.

https://codereview.chromium.org/2588843002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/front_end/gonzales/module.json (right):

https://codereview.chromium.org/2588843002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/front_end/gonzales/module.json:6:
"../common/TextRange.js",
On 2016/12/27 18:07:36, dgozman wrote:
> TextRange is a part of formatter_worker, which is a dependency. Why is it
> neccessary to mention TextRange here as well?

I've updated it so the DependencyPreprocessor checks for transitive implicit
dependencies so we don't need this.

https://codereview.chromium.org/2588843002/diff/120001/third_party/WebKit/Sou...
File
third_party/WebKit/Source/devtools/scripts/closure/closure_runner/src/org/chromium/devtools/compiler/Runner.java
(right):

https://codereview.chromium.org/2588843002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/scripts/closure/closure_runner/src/org/chromium/devtools/compiler/Runner.java:191:
private static class Flags {
On 2016/12/27 18:07:36, dgozman wrote:
> Why do we need all these classes (Flags, CompilerInstanceDescriptor, etc.)?
Can
> we just pass our coding conventions and let their runner handle command line
> flags?

Splitting this up into a separate CL.

https://codereview.chromium.org/2588843002/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/scripts/dependency_checker.py (right):

https://codereview.chromium.org/2588843002/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/scripts/dependency_checker.py:16: class
DependencyChecker(object):
On 2016/12/27 18:07:36, dgozman wrote:
> AFAIU, this is not a checker per se, but rather a preprocessor.

good suggestion, I've renamed the file + class.

[chenwilliam, 2017-01-05 19:35:26]

Description was changed from

==========
DevTools: speed up closure dependency checking

Compiling frontend runs in ~13 seconds (including dependency check) instead of
22 seconds.

Examples of the RegEx match:
http://regexr.com/3etk5

BUG=673445
==========

to

==========
DevTools: speed up closure dependency checking

Compiling frontend runs in ~13 seconds (including dependency check) instead of
22 seconds.

Examples of the RegEx match:
http://regexr.com/3etk5

The following CL needs to land before this CL lands:
https://codereview.chromium.org/2608043002

BUG=673445
==========

[chenwilliam, 2017-01-10 08:27:57]

Description was changed from

==========
DevTools: speed up closure dependency checking

Compiling frontend runs in ~13 seconds (including dependency check) instead of
22 seconds.

Examples of the RegEx match:
http://regexr.com/3etk5

The following CL needs to land before this CL lands:
https://codereview.chromium.org/2608043002

BUG=673445
==========

to

==========
DevTools: speed up closure dependency checking

Compiling frontend runs in ~13 seconds (including dependency check) instead of
22 seconds.

Examples of the RegEx match:
http://regexr.com/3etk5

The following CL needs to land before this CL lands:
https://codereview.chromium.org/2623743002/
BUG=673445
==========

[dgozman, 2017-01-10 21:47:35]

lgtm

https://codereview.chromium.org/2588843002/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/scripts/compile_frontend.py (right):

https://codereview.chromium.org/2588843002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/scripts/compile_frontend.py:239: if
descriptors.application[module_name].get('type', None) == 'worker':
FYI: this is never true anymore, so we should follow-up.

https://codereview.chromium.org/2588843002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/scripts/compile_frontend.py:316: command =
[arg.replace(devtools_frontend_path, temp_frontend_path) for arg in command]
Please double this is necessary.

https://codereview.chromium.org/2588843002/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/scripts/dependency_preprocessor.py
(right):

https://codereview.chromium.org/2588843002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/scripts/dependency_preprocessor.py:82: regex
= r'([^.]\b)' + namespace + r'(\b[^:])'
Let's have a comment that this should actually be
[^.]\s*\b     and      \b\s*[^:]
but we rely on clang-format never producing this.


We can do a single run, maybe it's faster?
[^.]\b(SDK|UI|Console)\b[^:]
\1$UndeclaredDependency_\2$$\3

https://codereview.chromium.org/2588843002/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/scripts/special_case_namespaces.json
(left):

https://codereview.chromium.org/2588843002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/scripts/special_case_namespaces.json:6:
"ui_lazy": "UI",
Change together with Runtime.js.

[chenwilliam, 2017-01-10 22:36:53]

The CQ bit was checked by chenwilliam@chromium.org

[chenwilliam, 2017-01-10 22:36:53]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/2588843002/#ps260001
(title: "re-insert comment")

[commit-bot: I haz the power, 2017-01-10 22:37:20]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[chenwilliam, 2017-01-10 23:08:56]

https://codereview.chromium.org/2588843002/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/scripts/compile_frontend.py (right):

https://codereview.chromium.org/2588843002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/scripts/compile_frontend.py:239: if
descriptors.application[module_name].get('type', None) == 'worker':
On 2017/01/10 21:47:35, dgozman wrote:
> FYI: this is never true anymore, so we should follow-up.

made a todo.

https://codereview.chromium.org/2588843002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/scripts/compile_frontend.py:316: command =
[arg.replace(devtools_frontend_path, temp_frontend_path) for arg in command]
On 2017/01/10 21:47:34, dgozman wrote:
> Please double this is necessary.

just double checked it's a full path.

https://codereview.chromium.org/2588843002/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/scripts/dependency_preprocessor.py
(right):

https://codereview.chromium.org/2588843002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/scripts/dependency_preprocessor.py:82: regex
= r'([^.]\b)' + namespace + r'(\b[^:])'
On 2017/01/10 21:47:35, dgozman wrote:
> Let's have a comment that this should actually be
> [^.]\s*\b     and      \b\s*[^:]
> but we rely on clang-format never producing this.
> 
> 
> We can do a single run, maybe it's faster?
> [^.]\b(SDK|UI|Console)\b[^:]
> \1$UndeclaredDependency_\2$$\3

Done. Changed to single run (~1 sec faster)

https://codereview.chromium.org/2588843002/diff/220001/third_party/WebKit/Sou...
File third_party/WebKit/Source/devtools/scripts/special_case_namespaces.json
(left):

https://codereview.chromium.org/2588843002/diff/220001/third_party/WebKit/Sou...
third_party/WebKit/Source/devtools/scripts/special_case_namespaces.json:6:
"ui_lazy": "UI",
On 2017/01/10 21:47:35, dgozman wrote:
> Change together with Runtime.js.

Done.

[commit-bot: I haz the power, 2017-01-11 01:20:48]

CQ is committing da patch.

Bot data: {"patchset_id": 260001, "attempt_start_ts": 1484087813095530,
"parent_rev": "59eb3dfeccb13a656ffcb1464d628b0b557af4cd", "commit_rev":
"2bc46a6dd144aa85e0aecd8f16631dc6bbdfa11e"}

[commit-bot: I haz the power, 2017-01-11 01:22:03]

Description was changed from

==========
DevTools: speed up closure dependency checking

Compiling frontend runs in ~13 seconds (including dependency check) instead of
22 seconds.

Examples of the RegEx match:
http://regexr.com/3etk5

The following CL needs to land before this CL lands:
https://codereview.chromium.org/2623743002/
BUG=673445
==========

to

==========
DevTools: speed up closure dependency checking

Compiling frontend runs in ~13 seconds (including dependency check) instead of
22 seconds.

Examples of the RegEx match:
http://regexr.com/3etk5

The following CL needs to land before this CL lands:
https://codereview.chromium.org/2623743002/
BUG=673445

Review-Url: https://codereview.chromium.org/2588843002
Cr-Commit-Position: refs/heads/master@{#442754}
Committed:
https://chromium.googlesource.com/chromium/src/+/2bc46a6dd144aa85e0aecd8f1663...
==========

[commit-bot: I haz the power, 2017-01-11 01:22:04]

Committed patchset #13 (id:260001) as
https://chromium.googlesource.com/chromium/src/+/2bc46a6dd144aa85e0aecd8f1663...