Chromium Code Reviews
Help | Chromium Project | Sign in
(2)

Issue 169743005: Faster run-bindings-tests (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 2 months ago by terry
Modified:
1 year, 2 months ago
CC:
blink-reviews, kojih, arv+blink, jsbell+bindings_chromium.org, sof, abarth, marja+watch_chromium.org, adamk+blink_chromium.org, haraken, Nate Chapin, Inactive
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Visibility:
Public.

Description

Faster run-bindings-tests This makes run-bindings-tests faster by doing more work in Python, instead of going out to the OS. Notably: calling Python compiler directly (reusing a compiler object), instead of launching separate processes, and avoiding a temporary file for interfaces_info. (Earlier versions had many refactoring changes, which improve support for Dart CG and other backends, which Nils broke off and committed separately.) R=nbarth Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169440

Patch Set 1 #

Patch Set 2 : Removed timings and fast is the only mode. #

Total comments: 8

Patch Set 3 : Cleanup #

Patch Set 4 : linter cleanup #

Patch Set 5 : linter cleanup #

Patch Set 6 : Merged and rebased #

Patch Set 7 : Integrated CL suggestions #

Patch Set 8 : merged #

Patch Set 9 : merged #

Patch Set 10 : Merged #

Patch Set 11 : Another merge #

Total comments: 19

Patch Set 12 : Merged with trunk #

Patch Set 13 : minor cleanup #

Patch Set 14 : minor cleanup #

Patch Set 15 : removed obsolete method #

Total comments: 8

Patch Set 16 : Changes from CR #

Total comments: 21

Patch Set 17 : Integrated CL comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -26 lines) Patch
A + Source/bindings/__init__.py View 3 4 5 6 7 8 9 10 11 12 0 chunks +-1 lines, --1 lines 0 comments Download
A + Source/bindings/scripts/__init__.py View 3 4 5 6 7 8 9 10 11 12 0 chunks +-1 lines, --1 lines 0 comments Download
M Tools/Scripts/webkitpy/bindings/main.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 9 chunks +35 lines, -28 lines 0 comments Download
Commit: CQ not working?

Messages

Total messages: 22 (0 generated)
terry
Faster dependency computation and parsing: - Fixed get_file_content being called twice; once for dependencies and ...
1 year, 2 months ago (2014-02-21 21:25:36 UTC) #1
Nils Barth (inactive)
Thanks Terry! I'm currently factoring compute_dependencies.py in these 3 CLs: https://codereview.chromium.org/171373006 https://codereview.chromium.org/173503009 https://codereview.chromium.org/174953005 ...so once ...
1 year, 2 months ago (2014-02-24 08:34:48 UTC) #2
Nils Barth (inactive)
Few more notes. Generally looks fine; once factoring of compute_dependencies lands and we switch the ...
1 year, 2 months ago (2014-02-24 08:38:55 UTC) #3
Nils Barth (inactive)
Some notes on recent changes. https://codereview.chromium.org/169743005/diff/30001/Source/bindings/scripts/unstable/idl_compiler.py File Source/bindings/scripts/unstable/idl_compiler.py (right): https://codereview.chromium.org/169743005/diff/30001/Source/bindings/scripts/unstable/idl_compiler.py#newcode65 Source/bindings/scripts/unstable/idl_compiler.py:65: write_header_and_cpp=code_generator_v8.write_header_and_cpp): I ultimately decided ...
1 year, 2 months ago (2014-02-25 07:29:59 UTC) #4
terry
On 2014/02/24 08:34:48, Nils Barth wrote: > Thanks Terry! > > I'm currently factoring compute_dependencies.py ...
1 year, 2 months ago (2014-02-27 22:55:59 UTC) #5
terry
Nils, I've sync'd to you changes and made the changes you've suggested. PTAL. Thanks again. ...
1 year, 2 months ago (2014-02-27 23:40:25 UTC) #6
terry
Nils I've re-merged with your latest round of checkins (removed more code as you moved ...
1 year, 2 months ago (2014-02-28 14:52:04 UTC) #7
Nils Barth (inactive)
Thanks for revisions Terry! Main change I'd suggest is to have an IdlCompiler class to ...
1 year, 2 months ago (2014-03-03 01:32:48 UTC) #8
Nils Barth (inactive)
Quick update on m0ar simplifications. https://chromiumcodereview.appspot.com/169743005/diff/30001/Source/bindings/scripts/compute_dependencies.py File Source/bindings/scripts/compute_dependencies.py (left): https://chromiumcodereview.appspot.com/169743005/diff/30001/Source/bindings/scripts/compute_dependencies.py#oldcode78 Source/bindings/scripts/compute_dependencies.py:78: parser.add_option('--interfaces-info-file', help='output pickle file') ...
1 year, 2 months ago (2014-03-03 06:45:48 UTC) #9
Nils Barth (inactive)
FYI: I've made substantial simplifications to compute_interfaces_info.py and r-b-t (main.py) in 2 CLs (below). Once ...
1 year, 2 months ago (2014-03-06 01:47:22 UTC) #10
Nils Barth (inactive)
*ping* Refactoring of IDL build is complete, so a rebase should simplify this CL, and ...
1 year, 2 months ago (2014-03-10 07:12:23 UTC) #11
Nils Barth (inactive)
All relevant refactoring CLs have landed, so once this CL is synced, it'll be quite ...
1 year, 2 months ago (2014-03-10 10:16:43 UTC) #12
terry
Nils, I've merged with your latest changes. The diffs are now minor. PTAL. https://codereview.chromium.org/169743005/diff/270001/Source/bindings/scripts/compute_interfaces_info.py File ...
1 year, 2 months ago (2014-03-13 19:58:14 UTC) #13
Nils Barth (inactive)
One revision, which makes this even simpler: we don't need a temporary file for |interfaces_info|, ...
1 year, 2 months ago (2014-03-14 01:01:36 UTC) #14
terry
PTAL https://chromiumcodereview.appspot.com/169743005/diff/350001/Source/bindings/scripts/compute_interfaces_info.py File Source/bindings/scripts/compute_interfaces_info.py (right): https://chromiumcodereview.appspot.com/169743005/diff/350001/Source/bindings/scripts/compute_interfaces_info.py#newcode271 Source/bindings/scripts/compute_interfaces_info.py:271: def compute(idl_files, interfaces_info_file, only_if_changed): Right with compute_interfaces_info no ...
1 year, 2 months ago (2014-03-14 16:12:54 UTC) #15
Nils Barth (inactive)
Thanks Terry! A few style nits, and I just noticed that we need to update ...
1 year, 2 months ago (2014-03-17 02:06:13 UTC) #16
terry
Thanks Nils, PTAL. https://chromiumcodereview.appspot.com/169743005/diff/370001/Tools/Scripts/webkitpy/bindings/main.py File Tools/Scripts/webkitpy/bindings/main.py (right): https://chromiumcodereview.appspot.com/169743005/diff/370001/Tools/Scripts/webkitpy/bindings/main.py#newcode36 Tools/Scripts/webkitpy/bindings/main.py:36: # for compute_dependencies and idl_compiler On ...
1 year, 2 months ago (2014-03-17 16:36:37 UTC) #17
Nils Barth (inactive)
LGTM, thanks! https://chromiumcodereview.appspot.com/169743005/diff/370001/Tools/Scripts/webkitpy/bindings/main.py File Tools/Scripts/webkitpy/bindings/main.py (right): https://chromiumcodereview.appspot.com/169743005/diff/370001/Tools/Scripts/webkitpy/bindings/main.py#newcode233 Tools/Scripts/webkitpy/bindings/main.py:233: self.idl_compiler = IdlCompilerV8(self.output_directory, On 2014/03/17 16:36:38, terry ...
1 year, 2 months ago (2014-03-18 09:23:15 UTC) #18
Nils Barth (inactive)
The CQ bit was checked by nbarth@chromium.org
1 year, 2 months ago (2014-03-18 09:23:21 UTC) #19
I haz the power - commit-bot
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/terry@google.com/169743005/380001
1 year, 2 months ago (2014-03-18 09:23:23 UTC) #20
I haz the power - commit-bot
Change committed as 169440
1 year, 2 months ago (2014-03-18 10:15:52 UTC) #21
Nils Barth (inactive)
1 year, 2 months ago (2014-03-19 01:03:55 UTC) #22
Message was sent while issue was closed.
On 2014/03/18 10:15:52, I haz the power (commit-bot) wrote:
> Change committed as 169440

Thanks Terry!
This is great -- cuts r-b-t time down to 1.4 seconds for me (from over 3
seconds),
with user time ~1s, and sys time .06 seconds (down from .48 seconds)!
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld ec887be