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

Upgrade V8 to 7.4.98 (kKeep fix) #1640

Merged
merged 2 commits into from Feb 7, 2019

Conversation

4 participants
@ry
Copy link
Collaborator

ry commented Feb 1, 2019

@ry ry force-pushed the v8_upgrade_kkeep branch from 6826f03 to 061a569 Feb 1, 2019

@ry ry force-pushed the v8_upgrade_kkeep branch 2 times, most recently from adb0011 to cf70997 Feb 1, 2019

@ry

This comment has been minimized.

Copy link
Collaborator Author

ry commented Feb 1, 2019

Basically no speed up, unfortunately:

> deno -v
deno: 0.2.9
v8: 7.2.502.16
> target/release/deno -v
deno: 0.2.9
v8: 7.4.0 (candidate)
> ./prebuilt/linux64/hyperfine --warmup 3 \
  "./target/release/deno  tests/003_relative_import.ts --recompile" \
  "deno tests/003_relative_import.ts --recompile"
Benchmark #1: ./target/release/deno  tests/003_relative_import.ts --recompile
  Time (mean ± σ):     384.1 ms ±   2.6 ms    [User: 591.2 ms, System: 35.2 ms]
  Range (min … max):   380.0 ms … 387.7 ms    10 runs

Benchmark #2: deno tests/003_relative_import.ts --recompile
  Time (mean ± σ):     423.0 ms ±   3.0 ms    [User: 640.4 ms, System: 30.4 ms]
  Range (min … max):   419.7 ms … 429.2 ms    10 runs

Summary
  './target/release/deno  tests/003_relative_import.ts --recompile' ran
    1.10 ± 0.01 times faster than 'deno tests/003_relative_import.ts --recompile'

Probably we need to exercise the TS compiler before snapshotting to get optimized code.

@ry

This comment has been minimized.

Copy link
Collaborator Author

ry commented Feb 1, 2019

Note that the patch we need was reverted earlier today: https://bugs.chromium.org/p/v8/issues/detail?id=8761#c6
We should wait until it lands again before landing this

@kitsonk

This comment has been minimized.

Copy link
Contributor

kitsonk commented Feb 2, 2019

Probably we need to exercise the TS compiler before snapshotting to get optimized code.

I looked at it before, it is semi tricky, we might have to do something silly like actually compile some code and emit it.

@ry ry force-pushed the v8_upgrade_kkeep branch from cf70997 to 27a7daa Feb 7, 2019

@ry ry force-pushed the v8_upgrade_kkeep branch from 27a7daa to 612b2b9 Feb 7, 2019

@ry ry changed the title Upgrade V8 to a1b431 (kKeep fix) Upgrade V8 to 7.4.98 (kKeep fix) Feb 7, 2019

@ry ry requested a review from piscisaureus Feb 7, 2019

@ry

This comment has been minimized.

Copy link
Collaborator Author

ry commented Feb 7, 2019

Fix has been landed in V8 7.4.98.

This patch is ready to land.

@piscisaureus
Copy link
Collaborator

piscisaureus left a comment

LGTM but double check if you wanted to commit the if-needed change.

@@ -255,8 +255,7 @@ def download_clang_format():

# Download clang by calling the clang update script.
def download_clang():
run(['python',
tp('v8/tools/clang/scripts/update.py'), '--if-needed'],
run(['python', tp('v8/tools/clang/scripts/update.py')],

This comment has been minimized.

@piscisaureus

piscisaureus Feb 7, 2019

Collaborator

No more --if-needed?

This comment has been minimized.

@ry

ry Feb 7, 2019

Author Collaborator

Dont think so / they seem to have removed it.

@ry ry merged commit 79b9534 into master Feb 7, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
license/cla Contributor License Agreement is signed.
Details

@ry ry deleted the v8_upgrade_kkeep branch Feb 7, 2019

ry added a commit to ry/deno that referenced this pull request Feb 9, 2019

v0.2.11
- Add deps to --info output (denoland#1720)
- Add --allow-read (denoland#1689)
- Add deno.isTTY() (denoland#1622)
- Add emojis to permission prompts (denoland#1684)
- Add basic WebAssembly support (denoland#1677)
- Add `NO_COLOR` support https://no-color.org/ (denoland#1716)
- Add color exceptions (denoland#1698)
- Fix: do not load cache files when recompile flag is set (denoland#1695)
- Upgrade V8 to 7.4.98 (denoland#1640)

ry added a commit that referenced this pull request Feb 9, 2019

v0.2.11
- Add deps to --info output (#1720)
- Add --allow-read (#1689)
- Add deno.isTTY() (#1622)
- Add emojis to permission prompts (#1684)
- Add basic WebAssembly support (#1677)
- Add `NO_COLOR` support https://no-color.org/ (#1716)
- Add color exceptions (#1698)
- Fix: do not load cache files when recompile flag is set (#1695)
- Upgrade V8 to 7.4.98 (#1640)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment