New issue
Advanced search Search tips

Issue 841466 link

Starred by 3 users
Status: Started
Owner:
hirosh...@chromium.org
Cc:
yangguo@chromium.org
leszeks@chromium.org
kouhei@chromium.org
mythria@chromium.org
neis@chromium.org
hirosh...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature

Blocked on:
issue 845285


Sign in to add a comment

Implement V8 Code Cache for module scripts, Blink-side

Project Member Reported by hirosh...@chromium.org, May 9 2018 
v8-side Issue: https://bugs.chromium.org/p/v8/issues/detail?id=7685

This issue tracks Blink-side implementation for V8 code cache for module scripts:
- Plumbs CachedMetadataHandler from loading to core/script to bindings/core/v8.
- Refactors related code, e.g. V8ScriptRunner where V8 code cache is implemented for classic scripts.
Project Member

Comment 1 by bugdroid1@chromium.org, May 9 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3faa281a9d683abe6ad1b544b4079c4f036173e9

commit 3faa281a9d683abe6ad1b544b4079c4f036173e9
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Wed May 09 21:03:04 2018

Remove SelectCompileFunction

To simplify code structure in V8ScriptRunner,
this CL flattens/inlines code around SelectCompileFunction.

This CL shouldn't change the behavior.

Bug: 841466
Change-Id: I5ec313b451c25417c8efdbd100edd46be5a002c1
Reviewed-on: https://chromium-review.googlesource.com/1045622
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Mythri Alle <mythria@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557315}
[modify] https://crrev.com/3faa281a9d683abe6ad1b544b4079c4f036173e9/third_party/blink/renderer/bindings/core/v8/v8_script_runner.cc
Project Member

Comment 2 by bugdroid1@chromium.org, May 9 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/28f42d2cc4c3e6736b992b54bcf17c75849ec4a9

commit 28f42d2cc4c3e6736b992b54bcf17c75849ec4a9
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Wed May 09 22:59:28 2018

Remove unused ScriptSourceCode::IsNull() and StartLine()

Bug: 841466
Change-Id: I0e98b5cd82150826330eb506bd2c973d87538388
Reviewed-on: https://chromium-review.googlesource.com/1048034
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557364}
[modify] https://crrev.com/28f42d2cc4c3e6736b992b54bcf17c75849ec4a9/third_party/blink/renderer/bindings/core/v8/script_source_code.h
Project Member

Comment 3 by bugdroid1@chromium.org, May 14 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e1352e6aff1cd10be78e0f85d54fb46f2d799936

commit e1352e6aff1cd10be78e0f85d54fb46f2d799936
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Mon May 14 23:03:06 2018

Make WebLocalFrame::ExecuteScriptInIsolatedWorld() single-script-only

This CL removes |num_sources| arguments of
WebLocalFrame::ExecuteScriptInIsolatedWorld() methods,
because |num_sources| is always 1.

This CL also renames WebLocalFrame::ExecuteScriptInIsolatedWorld()
with |results| argument to
WebLocalFrame::ExecuteScriptInIsolatedWorldAndReturnValue().

This CL shouldn't change the behavior.

This is preparation for
https://chromium-review.googlesource.com/721862.

Bug: 841466
Change-Id: I4a6b2d9da9bfd052cabe9147dac8c9f7818367c4
Reviewed-on: https://chromium-review.googlesource.com/1048323
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558510}
[modify] https://crrev.com/e1352e6aff1cd10be78e0f85d54fb46f2d799936/components/translate/content/renderer/translate_helper.cc
[modify] https://crrev.com/e1352e6aff1cd10be78e0f85d54fb46f2d799936/content/shell/test_runner/test_runner_for_specific_view.cc
[modify] https://crrev.com/e1352e6aff1cd10be78e0f85d54fb46f2d799936/third_party/blink/public/web/web_local_frame.h
[modify] https://crrev.com/e1352e6aff1cd10be78e0f85d54fb46f2d799936/third_party/blink/renderer/core/exported/web_frame_test.cc
[modify] https://crrev.com/e1352e6aff1cd10be78e0f85d54fb46f2d799936/third_party/blink/renderer/core/frame/web_local_frame_impl.cc
[modify] https://crrev.com/e1352e6aff1cd10be78e0f85d54fb46f2d799936/third_party/blink/renderer/core/frame/web_local_frame_impl.h

Comment 4 by hirosh...@chromium.org, May 21 2018

Blockedon: 845285
Project Member

Comment 5 by bugdroid1@chromium.org, May 26 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d00a9da08feb014e33bbe73e5692cbbd4331ed93

commit d00a9da08feb014e33bbe73e5692cbbd4331ed93
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Sat May 26 02:41:57 2018

Make ScriptController::ExecuteScriptInIsolatedWorld() single-script-only

This CL stops ScriptController::ExecuteScriptInIsolatedWorld()
receiving multiple scripts, and thus removes a FIXME
introduced in https://codereview.chromium.org/713743002.

This CL moves the code for executing multiple scripts
from ScriptController::ExecuteScriptInIsolatedWorld()
to pausable_script_executor.cc (the only caller that requires
multiple scripts support).

Bug: 841466
Change-Id: Iff36e910878398b314d99098cb8d111ecc8b048c
Reviewed-on: https://chromium-review.googlesource.com/721862
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562098}
[modify] https://crrev.com/d00a9da08feb014e33bbe73e5692cbbd4331ed93/third_party/blink/renderer/bindings/core/v8/activity_logger_test.cc
[modify] https://crrev.com/d00a9da08feb014e33bbe73e5692cbbd4331ed93/third_party/blink/renderer/bindings/core/v8/script_controller.cc
[modify] https://crrev.com/d00a9da08feb014e33bbe73e5692cbbd4331ed93/third_party/blink/renderer/bindings/core/v8/script_controller.h
[modify] https://crrev.com/d00a9da08feb014e33bbe73e5692cbbd4331ed93/third_party/blink/renderer/core/frame/pausable_script_executor.cc
[modify] https://crrev.com/d00a9da08feb014e33bbe73e5692cbbd4331ed93/third_party/blink/renderer/core/frame/web_local_frame_impl.cc
[modify] https://crrev.com/d00a9da08feb014e33bbe73e5692cbbd4331ed93/third_party/blink/renderer/core/xml/document_xml_tree_viewer.cc
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 7 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/781c15c373baee26d5447ff9157370411f61b18f

commit 781c15c373baee26d5447ff9157370411f61b18f
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Thu Jun 07 03:38:24 2018

Split V8 Code Cache implementation from V8ScriptRunner to V8CodeCache

Bug: 841466
Change-Id: I2c7a574aff3f40fe28aa5583fbec412a25869dfe
Reviewed-on: https://chromium-review.googlesource.com/1058674
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Mythri Alle <mythria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565167}
[modify] https://crrev.com/781c15c373baee26d5447ff9157370411f61b18f/third_party/blink/renderer/bindings/bindings.gni
[modify] https://crrev.com/781c15c373baee26d5447ff9157370411f61b18f/third_party/blink/renderer/bindings/core/v8/script_controller.cc
[modify] https://crrev.com/781c15c373baee26d5447ff9157370411f61b18f/third_party/blink/renderer/bindings/core/v8/script_streamer.cc
[modify] https://crrev.com/781c15c373baee26d5447ff9157370411f61b18f/third_party/blink/renderer/bindings/core/v8/script_streamer_test.cc
[add] https://crrev.com/781c15c373baee26d5447ff9157370411f61b18f/third_party/blink/renderer/bindings/core/v8/v8_code_cache.cc
[add] https://crrev.com/781c15c373baee26d5447ff9157370411f61b18f/third_party/blink/renderer/bindings/core/v8/v8_code_cache.h
[modify] https://crrev.com/781c15c373baee26d5447ff9157370411f61b18f/third_party/blink/renderer/bindings/core/v8/v8_script_runner.cc
[modify] https://crrev.com/781c15c373baee26d5447ff9157370411f61b18f/third_party/blink/renderer/bindings/core/v8/v8_script_runner.h
[modify] https://crrev.com/781c15c373baee26d5447ff9157370411f61b18f/third_party/blink/renderer/bindings/core/v8/v8_script_runner_test.cc
[modify] https://crrev.com/781c15c373baee26d5447ff9157370411f61b18f/third_party/blink/renderer/bindings/core/v8/worker_or_worklet_script_controller.cc
[modify] https://crrev.com/781c15c373baee26d5447ff9157370411f61b18f/third_party/blink/renderer/modules/cache_storage/cache.cc
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 11 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/95670db5fbc28116e19d00294b777ab2423fbf8f

commit 95670db5fbc28116e19d00294b777ab2423fbf8f
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Mon Jun 11 17:12:09 2018

Move logic for IsServedFromCacheStorage() to V8CodeCache

Bug: 841466
Change-Id: I4b08f6b67049ca22ac083c3b6cbb7954a5f1400f
Reviewed-on: https://chromium-review.googlesource.com/1058709
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Mythri Alle <mythria@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566037}
[modify] https://crrev.com/95670db5fbc28116e19d00294b777ab2423fbf8f/third_party/blink/renderer/bindings/core/v8/script_controller.cc
[modify] https://crrev.com/95670db5fbc28116e19d00294b777ab2423fbf8f/third_party/blink/renderer/bindings/core/v8/v8_code_cache.cc
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 14 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3ae0478ecad703b426cf8f4dad042de81225a89c

commit 3ae0478ecad703b426cf8f4dad042de81225a89c
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Thu Jun 14 18:42:10 2018

Remove Modulator::CompileModule()

This CL merges ModulatorImplBase::CompileModule() into
ModuleScript::Create() and removes Modulator::CompileModule(),
- Because ModuleScript will need more closer interaction with
  ScriptModule and V8CodeCache in bindings.
- Also this is a part of removing indirect layers around Modulator.

Bug: 845285, 841466
Change-Id: I4a2be4a816c4cc89a9ad57f8b0ec5a206000343c
Reviewed-on: https://chromium-review.googlesource.com/1065061
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567357}
[modify] https://crrev.com/3ae0478ecad703b426cf8f4dad042de81225a89c/third_party/blink/renderer/core/loader/modulescript/module_script_loader_test.cc
[modify] https://crrev.com/3ae0478ecad703b426cf8f4dad042de81225a89c/third_party/blink/renderer/core/script/modulator.h
[modify] https://crrev.com/3ae0478ecad703b426cf8f4dad042de81225a89c/third_party/blink/renderer/core/script/modulator_impl_base.cc
[modify] https://crrev.com/3ae0478ecad703b426cf8f4dad042de81225a89c/third_party/blink/renderer/core/script/modulator_impl_base.h
[modify] https://crrev.com/3ae0478ecad703b426cf8f4dad042de81225a89c/third_party/blink/renderer/core/script/module_map_test.cc
[modify] https://crrev.com/3ae0478ecad703b426cf8f4dad042de81225a89c/third_party/blink/renderer/core/script/module_script.cc
[modify] https://crrev.com/3ae0478ecad703b426cf8f4dad042de81225a89c/third_party/blink/renderer/core/testing/dummy_modulator.cc
[modify] https://crrev.com/3ae0478ecad703b426cf8f4dad042de81225a89c/third_party/blink/renderer/core/testing/dummy_modulator.h

Comment 10 by yangguo@chromium.org, Sep 20 

What is the status of this? Do modules benefit from code caching yet?

Comment 11 by hirosh...@chromium.org, Sep 20 

The code is not yet landed.
I've just restarted the implementation (that has been blocked due to Issue 845285 until Jun/Jul, and I was extremely busy in Jul/Aug for other issues). Planning to land in early Oct, hopefully.

Comment 12 by yangguo@chromium.org, Sep 20 

Thanks for the update!

Comment 13 by hirosh...@chromium.org, Sep 21 

Draft CL: https://chromium-review.googlesource.com/c/chromium/src/+/1237715

TODO:
- To confirm this is actually working, via local performance tests using https://github.com/GoogleChromeLabs/samples-module-loading-comparison.
- Add more unit tests?
►

Sign in to add a comment