New issue
Advanced search Search tips
Starred by 34 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Feature

Blocked on:
issue 719007



Sign in to add a comment
link

Issue 719172: Implicit caching of WebAssembly compilation artifacts

Reported by evan....@gmail.com, May 6 2017 Project Member

Issue description

What steps will reproduce the problem?
(1) Load http://webassembly.org/demo/Tanks/ in Chrome 58.0.3029.96
(2) Load http://webassembly.org/demo/Tanks/ in Firefox 53.0.2
(3) Compare load times

What is the expected result?

The time to load the demo in Chrome is similar to or better than the time to load the demo in Firefox.

What happens instead?

Chrome loads the demo in 11 seconds but Firefox loads the demo in 5 seconds (as measured by time-to-game). It appears that Chrome is failing to cache the compilation of the WebAssembly module. Capturing a flame graph in Chrome shows a function called "doNativeWasm" taking over 8 seconds before the game starts executing code. I'm assuming that's the WebAssembly module being compiled.
 

Comment 1 by birunt...@mohanathas.com, May 6 2017

Components: Blink>JavaScript>WebAssembly

Comment 2 by danno@chromium.org, May 7 2017

Cc: clemensh@chromium.org ahaas@chromium.org
Owner: bradnelson@chromium.org
Status: Assigned (was: Untriaged)

Comment 3 by gauravde...@gmail.com, Aug 5 2017

I see that time to first line of main (AOT) is same for my app with or without indexeddb (in latest canary M62). The module is getting loaded from indexeddb (after enabling the enable-webassembly structured cloning flag which is off by default) but the performance does not change. Wiping indexeddb clean does not help too.

Comment 4 by gauravde...@gmail.com, Aug 16 2017

Perhaps this feature does not work in WebWorker yet in latest Canary. (even after enabling the flag)

Comment 5 by nitish08...@gmail.com, Oct 6 2017

We too are facing this issue, Can Google Team  please prioritise this issue ?

Comment 6 by bradnelson@chromium.org, Nov 30 2017

Blockedon: 719007
Labels: -Pri-3 Pri-2

Comment 7 by pwnall@chromium.org, Nov 30 2017

#6: I was under the impression that  Issue 719007  (which I'm working on) is needed to get WASM module storage without the WASM structured cloning flag. While the current serialization format (used to serialize WASM modules when the flag is set) will not be supported in the final implementation, it should still be caching WASM compilation.

I browsed through code down this path, which seems to suggest that compiled output is being serialized.

v8::ValueSerializer::WriteValue
-> v8::internal::ValueSerializer::WriteObject
-> v8::internal::ValueSerializer::WriteWasmModule
-> v8::internal::wasm::NativeModuleSerializer::SerializeWasmModule
-> v8::internal::CodeSerializer::Serialize

If I'm correct, this bug can be fixed independently of 719007. If not, can you please let me know what I'm getting wrong?

Comment 8 by evan....@gmail.com, Jun 7 2018

 Issue 719007  is now marked as WontFix. What does that mean for the fate of this issue? It's still an extremely important issue for us at Figma because it's blocking us from giving our users a good experience.

Comment 9 by dschuff@chromium.org, Jun 7 2018

Cc: bbudge@chromium.org

Comment 10 by bhav...@superhuman.com, Jun 7 2018

This issue is also blocking us at Superhuman to deploy some of our web assembly modules that is meant to increase the performance. I am pretty sure there are a tonne of other products/projects waiting on this.

Thank you, appreciate it!
- BK

Sent via Superhuman ( https://sprh.mn/?vip=bhavesh@superhuman.com )

On Wed, Jun 06, 2018 at 5:37 PM, evan.… < monorail+v2.4223502460@chromium.org > wrote:

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

Owner: bbudge@chromium.org
We're looking at a different caching approach which we think will work better for developers and may be more likely to find support in other browsers as well. I will create the tracking bug for this work soon and reference it here.

Comment 12 by c...@datacivilization.com, Jun 10 2018

That sounds promising. Is it already in the Chrome Canary? If not, which Chrome release (which month/year) would be the candidate? I think this is a crucial feature.

I would like to remind you the defunct NaCl did a better job of caching the compiled code. The Webassembly is supposed to do better, isn't it?

Comment 13 by bugdroid1@chromium.org, Aug 16

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0155acba99c2fee96a227607eb6bc2b1f49bd16a

commit 0155acba99c2fee96a227607eb6bc2b1f49bd16a
Author: Bill Budge <bbudge@chromium.org>
Date: Thu Aug 16 16:27:02 2018

[blink] Only create metadata handler for document RawResources.

- RawResource is used for several non-text resources, as well as
  document resources. The cached metadata handling has been set up
  for a document's inline scripts. This makes it difficult to cache
  metadata properly for other RawResources, such as WebAssembly
  modules. This change only creates SourceKeyedMetadataHandler for
  RawResources whose type is kMainResource.
- Renames RawResource::CacheHandler to InlineScriptCacheHandler so
  it doesn't hide the Resource::CacheHandler method, and so we can
  later add other kinds of metadata handlers.

Bug: chromium:719172
Change-Id: I53f361a0df48dc9d73780971e3408dfff08de518
Reviewed-on: https://chromium-review.googlesource.com/1173496
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Commit-Queue: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583684}
[modify] https://crrev.com/0155acba99c2fee96a227607eb6bc2b1f49bd16a/third_party/blink/renderer/core/loader/document_loader.cc
[modify] https://crrev.com/0155acba99c2fee96a227607eb6bc2b1f49bd16a/third_party/blink/renderer/platform/loader/fetch/raw_resource.cc
[modify] https://crrev.com/0155acba99c2fee96a227607eb6bc2b1f49bd16a/third_party/blink/renderer/platform/loader/fetch/raw_resource.h

Comment 14 by bugdroid1@chromium.org, Aug 17

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

commit cc081ca9aefcc65fa4dcbbeb0a7c80c0f5e5421f
Author: Bill Budge <bbudge@chromium.org>
Date: Fri Aug 17 20:01:00 2018

[blink] Add a ScriptCachedMetadataHandler class.

- Moves ScriptResource::SingleCachedMetadataHandlerImpl out of
  ScriptResource and into the blink namespace so it can be used by other
  script-like resources.
- Creates this CacheHandler type for RawResource type kRaw, which will
  be needed for WASM modules.

Bug: chromium:719172
Change-Id: I34a2740310d2d3f4ccb0cea1bb664db0b9c53f5e
Reviewed-on: https://chromium-review.googlesource.com/1174903
Commit-Queue: Bill Budge <bbudge@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584159}
[modify] https://crrev.com/cc081ca9aefcc65fa4dcbbeb0a7c80c0f5e5421f/third_party/blink/renderer/core/loader/resource/script_resource.cc
[modify] https://crrev.com/cc081ca9aefcc65fa4dcbbeb0a7c80c0f5e5421f/third_party/blink/renderer/core/loader/resource/script_resource.h
[modify] https://crrev.com/cc081ca9aefcc65fa4dcbbeb0a7c80c0f5e5421f/third_party/blink/renderer/platform/loader/BUILD.gn
[modify] https://crrev.com/cc081ca9aefcc65fa4dcbbeb0a7c80c0f5e5421f/third_party/blink/renderer/platform/loader/fetch/raw_resource.cc
[modify] https://crrev.com/cc081ca9aefcc65fa4dcbbeb0a7c80c0f5e5421f/third_party/blink/renderer/platform/loader/fetch/raw_resource.h
[add] https://crrev.com/cc081ca9aefcc65fa4dcbbeb0a7c80c0f5e5421f/third_party/blink/renderer/platform/loader/fetch/script_cached_metadata_handler.cc
[add] https://crrev.com/cc081ca9aefcc65fa4dcbbeb0a7c80c0f5e5421f/third_party/blink/renderer/platform/loader/fetch/script_cached_metadata_handler.h

Comment 15 by bugdroid1@chromium.org, Sep 28

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

commit d64546bc9ceeeac31127eca8b64b4722c3ebb8d4
Author: Bill Budge <bbudge@chromium.org>
Date: Fri Sep 28 16:06:47 2018

[caching] Add WebAssembly cache

- Modifies Mojo code caching API to specify JS or WASM cache.
- Adds WebAssembly GeneratedCodeCache.

Bug: chromium:719172
Change-Id: Ie7dec8112a8e6cea7c576e2e502683bba0bb3d4a
Reviewed-on: https://chromium-review.googlesource.com/1235283
Commit-Queue: Bill Budge <bbudge@chromium.org>
Reviewed-by: Mythri Alle <mythria@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595107}
[modify] https://crrev.com/d64546bc9ceeeac31127eca8b64b4722c3ebb8d4/content/browser/browsing_data/storage_partition_http_cache_data_remover.cc
[modify] https://crrev.com/d64546bc9ceeeac31127eca8b64b4722c3ebb8d4/content/browser/code_cache/generated_code_cache_context.cc
[modify] https://crrev.com/d64546bc9ceeeac31127eca8b64b4722c3ebb8d4/content/browser/code_cache/generated_code_cache_context.h
[modify] https://crrev.com/d64546bc9ceeeac31127eca8b64b4722c3ebb8d4/content/browser/renderer_host/code_cache_host_impl.cc
[modify] https://crrev.com/d64546bc9ceeeac31127eca8b64b4722c3ebb8d4/content/browser/renderer_host/code_cache_host_impl.h
[modify] https://crrev.com/d64546bc9ceeeac31127eca8b64b4722c3ebb8d4/content/browser/storage_partition_impl_unittest.cc
[modify] https://crrev.com/d64546bc9ceeeac31127eca8b64b4722c3ebb8d4/content/renderer/loader/code_cache_loader_impl.cc
[modify] https://crrev.com/d64546bc9ceeeac31127eca8b64b4722c3ebb8d4/content/renderer/loader/code_cache_loader_impl.h
[modify] https://crrev.com/d64546bc9ceeeac31127eca8b64b4722c3ebb8d4/content/renderer/renderer_blink_platform_impl.cc
[modify] https://crrev.com/d64546bc9ceeeac31127eca8b64b4722c3ebb8d4/content/renderer/renderer_blink_platform_impl.h
[modify] https://crrev.com/d64546bc9ceeeac31127eca8b64b4722c3ebb8d4/third_party/blink/public/mojom/loader/code_cache.mojom
[modify] https://crrev.com/d64546bc9ceeeac31127eca8b64b4722c3ebb8d4/third_party/blink/public/platform/code_cache_loader.h
[modify] https://crrev.com/d64546bc9ceeeac31127eca8b64b4722c3ebb8d4/third_party/blink/public/platform/platform.h
[modify] https://crrev.com/d64546bc9ceeeac31127eca8b64b4722c3ebb8d4/third_party/blink/renderer/platform/loader/fetch/resource.cc
[modify] https://crrev.com/d64546bc9ceeeac31127eca8b64b4722c3ebb8d4/third_party/blink/renderer/platform/loader/fetch/resource.h
[modify] https://crrev.com/d64546bc9ceeeac31127eca8b64b4722c3ebb8d4/third_party/blink/renderer/platform/loader/fetch/resource_loader.cc
[modify] https://crrev.com/d64546bc9ceeeac31127eca8b64b4722c3ebb8d4/third_party/blink/renderer/platform/loader/fetch/resource_loader.h
[modify] https://crrev.com/d64546bc9ceeeac31127eca8b64b4722c3ebb8d4/third_party/blink/renderer/platform/loader/fetch/resource_test.cc
[modify] https://crrev.com/d64546bc9ceeeac31127eca8b64b4722c3ebb8d4/third_party/blink/renderer/platform/loader/fetch/source_keyed_cached_metadata_handler_test.cc
[modify] https://crrev.com/d64546bc9ceeeac31127eca8b64b4722c3ebb8d4/third_party/blink/renderer/platform/testing/code_cache_loader_mock.cc
[modify] https://crrev.com/d64546bc9ceeeac31127eca8b64b4722c3ebb8d4/third_party/blink/renderer/platform/testing/code_cache_loader_mock.h

Comment 16 by kbr@chromium.org, Sep 28

Great work Bill! Very exciting!

Comment 17 by bugdroid1@chromium.org, Oct 4

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

commit f27eb2437ce09336e6c175965e72021ee0800b6d
Author: Bill Budge <bbudge@chromium.org>
Date: Thu Oct 04 18:45:41 2018

[caching] Allow WebAssembly fetches to use code cache.

Fixes a few problems preventing the WASM cache from working.
- Routes raw resource metadata to the WASM code cache.
- Separates JS and WASM generated code caches. They can't share
  a directory.
- Allow cached metadata to be set on RawResources.

Bug: chromium:719172
Change-Id: I0f3709340948bb25d134504bb03632eb1ce24196
Reviewed-on: https://chromium-review.googlesource.com/c/1255963
Commit-Queue: Bill Budge <bbudge@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Mythri Alle <mythria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596779}
[modify] https://crrev.com/f27eb2437ce09336e6c175965e72021ee0800b6d/content/browser/code_cache/generated_code_cache_context.cc
[modify] https://crrev.com/f27eb2437ce09336e6c175965e72021ee0800b6d/third_party/blink/renderer/platform/loader/fetch/raw_resource.cc
[modify] https://crrev.com/f27eb2437ce09336e6c175965e72021ee0800b6d/third_party/blink/renderer/platform/loader/fetch/resource_loader.cc
[modify] https://crrev.com/f27eb2437ce09336e6c175965e72021ee0800b6d/third_party/blink/renderer/platform/loader/fetch/resource_loader.h

Comment 18 by bugdroid1@chromium.org, Oct 5

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/ca3220c64954260da79d23c8566ec5e9dbe57f96

commit ca3220c64954260da79d23c8566ec5e9dbe57f96
Author: Bill Budge <bbudge@chromium.org>
Date: Fri Oct 05 21:24:46 2018

[wasm] Expose function IsSupportedVersion

- Exposes IsSupportedVersion function which compares serialized
  version to current Wasm version.
- Tweaks the comments on serialization to match the code.

Bug: chromium:719172
Change-Id: I76df9605aee16fd98cd82b54dba2e9acbd56b41b
Reviewed-on: https://chromium-review.googlesource.com/c/1265141
Reviewed-by: Ben Smith <binji@chromium.org>
Commit-Queue: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56420}
[modify] https://crrev.com/ca3220c64954260da79d23c8566ec5e9dbe57f96/src/wasm/wasm-serialization.cc
[modify] https://crrev.com/ca3220c64954260da79d23c8566ec5e9dbe57f96/src/wasm/wasm-serialization.h

Comment 19 by bugdroid1@chromium.org, Oct 12

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/9ecd4545fe424146f0a5fa5dbcaccd775048131a

commit 9ecd4545fe424146f0a5fa5dbcaccd775048131a
Author: Bill Budge <bbudge@chromium.org>
Date: Fri Oct 12 21:34:45 2018

[api] Add WebAssembly caching API

- Adds embedder callback to notify fully tiered compilation is finished,
  returning a WasmCompiledModule for serialization.
- Adds function to pass previously compiled bytes into WASM streaming
  compilation, for deserialization.
- Plumbs this API through StreamingDecoder.

Bug: chromium:719172
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: Ibe376f3a8ccfa90fda730ef4ff6628a1532da45c
Reviewed-on: https://chromium-review.googlesource.com/c/1252884
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56617}
[modify] https://crrev.com/9ecd4545fe424146f0a5fa5dbcaccd775048131a/include/v8.h
[modify] https://crrev.com/9ecd4545fe424146f0a5fa5dbcaccd775048131a/src/wasm/module-compiler.cc
[modify] https://crrev.com/9ecd4545fe424146f0a5fa5dbcaccd775048131a/src/wasm/module-compiler.h
[modify] https://crrev.com/9ecd4545fe424146f0a5fa5dbcaccd775048131a/src/wasm/streaming-decoder.cc
[modify] https://crrev.com/9ecd4545fe424146f0a5fa5dbcaccd775048131a/src/wasm/streaming-decoder.h
[modify] https://crrev.com/9ecd4545fe424146f0a5fa5dbcaccd775048131a/src/wasm/wasm-js.cc
[modify] https://crrev.com/9ecd4545fe424146f0a5fa5dbcaccd775048131a/src/wasm/wasm-js.h
[modify] https://crrev.com/9ecd4545fe424146f0a5fa5dbcaccd775048131a/src/wasm/wasm-objects.h
[modify] https://crrev.com/9ecd4545fe424146f0a5fa5dbcaccd775048131a/test/cctest/wasm/test-streaming-compilation.cc
[modify] https://crrev.com/9ecd4545fe424146f0a5fa5dbcaccd775048131a/test/unittests/wasm/streaming-decoder-unittest.cc

Comment 20 by bugdroid1@chromium.org, Oct 25

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/9fa085e59a7de104deb550112784a359f80b8991

commit 9fa085e59a7de104deb550112784a359f80b8991
Author: Bill Budge <bbudge@chromium.org>
Date: Thu Oct 25 15:56:24 2018

[wasm] When bypassing compilation, deserialize in a context

- Moves call to DeserializeNativeModule into SaveContext to avoid
  a crash in IsWasmCodegenAllowed.

Bug: chromium:719172
Change-Id: Idd367824a325fc684f29e335b0c07e515f9fdad3
Reviewed-on: https://chromium-review.googlesource.com/c/1298375
Commit-Queue: Bill Budge <bbudge@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56997}
[modify] https://crrev.com/9fa085e59a7de104deb550112784a359f80b8991/src/wasm/module-compiler.cc

Comment 21 by hablich@chromium.org, Oct 26

Labels: -Type-Bug Type-Feature
Summary: Implicit caching of WebAssembly compilation artifacts (was: Chrome fails to cache WebAssembly compilation)

Comment 22 by bugdroid1@chromium.org, Nov 20

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4d028f1266f84e97733b73c8ffe451e529a3bf01

commit 4d028f1266f84e97733b73c8ffe451e529a3bf01
Author: Bill Budge <bbudge@chromium.org>
Date: Tue Nov 20 15:05:12 2018

[WebAssembly] Add blink::feature kWasmCodeCache.

- Adds kWasmCodeCache to blink::features.
- Checks if enabled before creating the cache in the browser.
- Checks if enabled before fetching from cache in renderer.

This is patterned after the Javascript IsolatedCodeCache feature.

Bug: chromium:719172
Change-Id: I1ffea5d8029b2b550c45e3ba25821889c7c2183c
Reviewed-on: https://chromium-review.googlesource.com/c/1341071
Commit-Queue: Bill Budge <bbudge@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Mythri Alle <mythria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609705}
[modify] https://crrev.com/4d028f1266f84e97733b73c8ffe451e529a3bf01/content/browser/code_cache/generated_code_cache_context.cc
[modify] https://crrev.com/4d028f1266f84e97733b73c8ffe451e529a3bf01/content/browser/storage_partition_impl_unittest.cc
[modify] https://crrev.com/4d028f1266f84e97733b73c8ffe451e529a3bf01/content/child/runtime_features.cc
[modify] https://crrev.com/4d028f1266f84e97733b73c8ffe451e529a3bf01/third_party/blink/common/features.cc
[modify] https://crrev.com/4d028f1266f84e97733b73c8ffe451e529a3bf01/third_party/blink/public/common/features.h
[modify] https://crrev.com/4d028f1266f84e97733b73c8ffe451e529a3bf01/third_party/blink/public/platform/web_runtime_features.h
[modify] https://crrev.com/4d028f1266f84e97733b73c8ffe451e529a3bf01/third_party/blink/renderer/platform/exported/web_runtime_features.cc
[modify] https://crrev.com/4d028f1266f84e97733b73c8ffe451e529a3bf01/third_party/blink/renderer/platform/loader/fetch/resource_loader.cc
[modify] https://crrev.com/4d028f1266f84e97733b73c8ffe451e529a3bf01/third_party/blink/renderer/platform/runtime_enabled_features.json5

Comment 23 by bugdroid1@chromium.org, Dec 21

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/fc479d516b935615a4317d550d500ed87915ce03

commit fc479d516b935615a4317d550d500ed87915ce03
Author: Bill Budge <bbudge@chromium.org>
Date: Fri Dec 21 23:32:29 2018

[api] Change Wasm ModuleCompiled notification

- Removes ModuleCompiledCallback typedef and Set function.
- Adds WasmStreaming::Client abstraction and Set function.

Bug: chromium:719172
Change-Id: I8a207b628394a7660bda73cde560da1e461248a7
Reviewed-on: https://chromium-review.googlesource.com/c/1377450
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58454}
[modify] https://crrev.com/fc479d516b935615a4317d550d500ed87915ce03/include/v8.h
[modify] https://crrev.com/fc479d516b935615a4317d550d500ed87915ce03/src/wasm/streaming-decoder.cc
[modify] https://crrev.com/fc479d516b935615a4317d550d500ed87915ce03/src/wasm/wasm-js.cc

Comment 24 by bbudge@chromium.org, Jan 8

Status: Started (was: Assigned)

Comment 25 by bugdroid1@chromium.org, Jan 10

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/bb46048ad7d6da6322241fc12a8d03bf61319ee8

commit bb46048ad7d6da6322241fc12a8d03bf61319ee8
Author: Andreas Haas <ahaas@chromium.org>
Date: Thu Jan 10 14:10:52 2019

[wasm] Notify streaming decoder when the native module is created

Originally, the NativeModule and the WasmModuleObject were created
together, and the streaming decoder was notified after the
WasmModuleObject was created. A recent CL (https://crrev.com/c/1402544),
however, changed that.  The NativeModule gets created before compilation
starts, the WasmModuleObject, however, gets created after compilation.

The streaming decoder only needs the NativeModule to register a callback
before compilation. Therefore this CL we change the notification of the
streaming decoder to receive only the NativeModule, not the
WasmModuleObject, before starting compilation.

R=clemensh@chromium.org
CC=bbudge@chromium.org

Bug: chromium:719172
Change-Id: I4ad879e4ebd2d88174d7e2a0c6359f2836926763
Reviewed-on: https://chromium-review.googlesource.com/c/1404442
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58703}
[modify] https://crrev.com/bb46048ad7d6da6322241fc12a8d03bf61319ee8/src/wasm/module-compiler.cc
[modify] https://crrev.com/bb46048ad7d6da6322241fc12a8d03bf61319ee8/src/wasm/streaming-decoder.cc
[modify] https://crrev.com/bb46048ad7d6da6322241fc12a8d03bf61319ee8/src/wasm/streaming-decoder.h

Comment 26 by bugdroid, Jan 28

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

commit fc908613f8c0c8e60e99b33dd37d82007725bc89
Author: Bill Budge <bbudge@chromium.org>
Date: Mon Jan 28 22:06:26 2019

[caching] Use V8 WASM caching API in Blink

- Removes MainThread restriction for using Mojo caching API. WebAssembly
  streaming instantiation runs off the main thread.
- Adds trace events for streaming instantiation events of interest, which
  are disabled by default.
- Uses the Mojo caching API for WebAssembly streaming compilation, and
  caches modules over 128 KB in size.
- Adds web tests that use the tracing events to make sure caching is
  working correctly.
- Adds two .wasm files, above and below the caching threshold size,
  (128 KB).

Bug: chromium:719172
Change-Id: Iaf7e2fa96e9bb441bda18b780ae41794af119900
Reviewed-on: https://chromium-review.googlesource.com/c/1260209
Commit-Queue: Bill Budge <bbudge@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Mythri Alle <mythria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626755}

Comment 27 by kbr@chromium.org, Jan 28

The progress on WASM caching is tremendously exciting!

Comment 28 by bbudge@chromium.org, Jan 29

Thanks Ken. Assuming it sticks, users should be able to experiment with this in the next Canary with the --wasm-code-cache flag. In addition:

1) The wasm module must be compiled/instantiated with the streaming API.
2) The fetch response must have the Content-Type: application/wasm header.
3) The fetch response must have the Last-Modified header set to a fixed value so we know the module hasn't been changed.
4) The WASM module must be over 128K bytes in length for us to try to cache it.

The first fetch of the module should cache the compiled module. The performance improvement comes on the second fetch. I was able to see a difference with modules of just 1MB on my laptop.

Future work will be to start a Finch experiment, and then to enable this by default if all goes well.

Comment 29 by he...@jayphelps.com, Jan 29

Very cool. Thanks for the update. Will it work with custom constructed
Response objects from a Service Worker, or only directly with real fetch()
Responses? Eg if a Service Worker does some sort of stream processing to
generate the final Wasm binary with a custom Response + Readable stream.

Comment 30 by bugdroid, Jan 29

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/368bf36e00001342867a1a9ca69fa8b3e510457a

commit 368bf36e00001342867a1a9ca69fa8b3e510457a
Author: Clemens Hammacher <clemensh@chromium.org>
Date: Tue Jan 29 15:24:59 2019

[wasm] Remove strong reference in TopTierCompiledCallback

Registered callbacks should not keep the NativeModule alive. Otherwise,
tiering will always run to completion, even if the NativeModule is not
being used any more.
This change can cause the callback to not be called if the module dies
before it finishes top-tier compilation. This is the desired behaviour.

R=ahaas@chromium.org
CC=titzer@chromium.org, bbudge@chromium.org

Bug:  v8:8689 , chromium:719172
Change-Id: Ide9d639f465497c3ed3439c7ce25c76dceeb97eb
Reviewed-on: https://chromium-review.googlesource.com/c/1435937
Reviewed-by: Bill Budge <bbudge@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59175}
[modify] https://crrev.com/368bf36e00001342867a1a9ca69fa8b3e510457a/src/wasm/streaming-decoder.cc

Comment 31 by bugdroid, Jan 29

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8b7842262ee1239b1f3ae20b9c851748ef0b9a8b

commit 8b7842262ee1239b1f3ae20b9c851748ef0b9a8b
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Tue Jan 29 18:09:27 2019

Revert "[caching] Use V8 WASM caching API in Blink"

This reverts commit fc908613f8c0c8e60e99b33dd37d82007725bc89.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 626755 as the
culprit for flakes in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/flake/flake-culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyQwsSDEZsYWtlQ3VscHJpdCIxY2hyb21pdW0vZmM5MDg2MTNmOGMwYzhlNjBlOTliMzNkZDM3ZDgyMDA3NzI1YmM4OQw

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.linux/Linux%20Tests%20%28dbg%29%281%29/77658

Sample Failed Step: webkit_layout_tests

Sample Flaky Test: http/tests/devtools/wasm-isolated-code-cache/wasm-cache-test.js

Original change's description:
> [caching] Use V8 WASM caching API in Blink
> 
> - Removes MainThread restriction for using Mojo caching API. WebAssembly
>   streaming instantiation runs off the main thread.
> - Adds trace events for streaming instantiation events of interest, which
>   are disabled by default.
> - Uses the Mojo caching API for WebAssembly streaming compilation, and
>   caches modules over 128 KB in size.
> - Adds web tests that use the tracing events to make sure caching is
>   working correctly.
> - Adds two .wasm files, above and below the caching threshold size,
>   (128 KB).
> 
> Bug: chromium:719172
> Change-Id: Iaf7e2fa96e9bb441bda18b780ae41794af119900
> Reviewed-on: https://chromium-review.googlesource.com/c/1260209
> Commit-Queue: Bill Budge <bbudge@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Alexei Filippov <alph@chromium.org>
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Reviewed-by: Mythri Alle <mythria@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#626755}

Change-Id: Ib2d9616727542fe44485dec5aac924c5ef6a803e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:719172
Reviewed-on: https://chromium-review.googlesource.com/c/1443540
Cr-Commit-Position: refs/heads/master@{#627064}
[modify] https://crrev.com/8b7842262ee1239b1f3ae20b9c851748ef0b9a8b/third_party/blink/renderer/bindings/core/v8/v8_wasm_response_extensions.cc
[modify] https://crrev.com/8b7842262ee1239b1f3ae20b9c851748ef0b9a8b/third_party/blink/renderer/devtools/front_end/timeline/TimelineUIUtils.js
[modify] https://crrev.com/8b7842262ee1239b1f3ae20b9c851748ef0b9a8b/third_party/blink/renderer/devtools/front_end/timeline_model/TimelineModel.js
[modify] https://crrev.com/8b7842262ee1239b1f3ae20b9c851748ef0b9a8b/third_party/blink/renderer/platform/loader/fetch/resource_loader.cc
[modify] https://crrev.com/8b7842262ee1239b1f3ae20b9c851748ef0b9a8b/third_party/blink/web_tests/SlowTests
[modify] https://crrev.com/8b7842262ee1239b1f3ae20b9c851748ef0b9a8b/third_party/blink/web_tests/VirtualTestSuites
[delete] https://crrev.com/36989a28fae71201dde1bc16588aef4e457abef1/third_party/blink/web_tests/http/tests/devtools/wasm-isolated-code-cache/wasm-cache-test-expected.txt
[delete] https://crrev.com/36989a28fae71201dde1bc16588aef4e457abef1/third_party/blink/web_tests/http/tests/devtools/wasm-isolated-code-cache/wasm-cache-test.js
[delete] https://crrev.com/36989a28fae71201dde1bc16588aef4e457abef1/third_party/blink/web_tests/http/tests/wasm/resources/large.wasm
[modify] https://crrev.com/8b7842262ee1239b1f3ae20b9c851748ef0b9a8b/third_party/blink/web_tests/http/tests/wasm/resources/load-wasm.php
[delete] https://crrev.com/36989a28fae71201dde1bc16588aef4e457abef1/third_party/blink/web_tests/http/tests/wasm/resources/small.wasm
[delete] https://crrev.com/36989a28fae71201dde1bc16588aef4e457abef1/third_party/blink/web_tests/http/tests/wasm/resources/wasm-cache-iframe.html
[delete] https://crrev.com/36989a28fae71201dde1bc16588aef4e457abef1/third_party/blink/web_tests/virtual/wasm-site-isolated-code-cache/http/tests/devtools/wasm-isolated-code-cache/README.txt
[delete] https://crrev.com/36989a28fae71201dde1bc16588aef4e457abef1/third_party/blink/web_tests/virtual/wasm-site-isolated-code-cache/http/tests/devtools/wasm-isolated-code-cache/wasm-cache-test-expected.txt

Comment 32 by kbr@chromium.org, Jan 29

Unclear how many failures were detected by FindIt before doing the auto-revert, but here's one:

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Tests%20%28dbg%29%281%29/77658

Failing shard:
https://chromium-swarm.appspot.com/task?id=42b2f4f68ab08c10&refresh=10&show_raw=1

Log:
07:16:10.373 4795 worker/0 http/tests/devtools/wasm-isolated-code-cache/wasm-cache-test.js crashed, (stderr lines):
07:16:10.373 4795   
07:16:10.373 4795   
07:16:10.373 4795   #
07:16:10.373 4795   # Fatal error in ../../v8/src/v8memory.h, line 20
07:16:10.388 4795   # Debug check failed: 0 == addr & (alignof(T) - 1) (0 vs. 4).
07:16:10.388 4795   #
07:16:10.388 4795   #
07:16:10.388 4795   #
07:16:10.388 4795   #FailureMessage Object: 0x7ffe4d0bb280#0 0x7f27937036d1 base::debug::CollectStackTrace()
07:16:10.388 4795   #1 0x7f279354a44d base::debug::StackTrace::StackTrace()
07:16:10.388 4795   #2 0x7f279354a405 base::debug::StackTrace::StackTrace()
07:16:10.388 4795   #3 0x7f27870019e7 gin::(anonymous namespace)::PrintStackTrace()
07:16:10.388 4795   #4 0x7f276e5b9168 V8_Fatal()
07:16:10.388 4795   #5 0x7f276e5b8ed5 v8::base::(anonymous namespace)::DefaultDcheckHandler()
07:16:10.388 4795   #6 0x7f2786641331 v8::TickSample::Init()
07:16:10.388 4795   #7 0x7f2786641b7e v8::internal::TickSample::Init()
07:16:10.388 4795   #8 0x7f278661b092 v8::internal::CpuSampler::SampleStack()
07:16:10.388 4795   #9 0x7f27869667dd v8::sampler::SamplerManager::DoSample()
07:16:10.388 4795   #10 0x7f2786966841 v8::sampler::SignalHandler::HandleProfilerSignal()
07:16:10.388 4795   #11 0x7f2778639330 <unknown>
07:16:10.388 4795   #12 0x7ffe4d1b1d26 ([vdso]+0xd25)
07:16:10.388 4795   #13 0x7f277791151d __clock_gettime
07:16:10.388 4795   #14 0x7f276e5bb1c9 v8::base::TimeTicks::Now()
07:16:10.388 4795   #15 0x7f2785d9638b v8::base::ElapsedTimer::Start()
07:16:10.388 4795   #16 0x7f2785ec8771 v8::internal::Compiler::Compile()
07:16:10.388 4795   #17 0x7f2785ec94c3 v8::internal::Compiler::Compile()
07:16:10.388 4795   #18 0x7f27866a515b v8::internal::__RT_impl_Runtime_CompileLazy()
07:16:10.389 4795   #19 0x7f2786d03eb2 <unknown>
07:16:10.391 4720 [2457/4233] http/tests/devtools/wasm-isolated-code-cache/wasm-cache-test.js failed unexpectedly (renderer crashed)


Here were the flakiness dashboard results at the time:
https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=webkit_layout_tests&tests=http/tests/devtools/wasm-isolated-code-cache/wasm-cache-test.js

Comment 33 by bugdroid, Jan 30

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7aca1c5411ca2106eb4793fa3a23beb89fd9d778

commit 7aca1c5411ca2106eb4793fa3a23beb89fd9d778
Author: Bill Budge <bbudge@chromium.org>
Date: Wed Jan 30 18:57:29 2019

Reland "[caching] Use V8 WASM caching API in Blink"

This is a reland of fc908613f8c0c8e60e99b33dd37d82007725bc89

Since it's unchanged, landing with t-b-r

TBR=haraken@chromium.org,alph@chromium.org,kinuko@chromium.org

Original change's description:
> [caching] Use V8 WASM caching API in Blink
>
> - Removes MainThread restriction for using Mojo caching API. WebAssembly
>   streaming instantiation runs off the main thread.
> - Adds trace events for streaming instantiation events of interest, which
>   are disabled by default.
> - Uses the Mojo caching API for WebAssembly streaming compilation, and
>   caches modules over 128 KB in size.
> - Adds web tests that use the tracing events to make sure caching is
>   working correctly.
> - Adds two .wasm files, above and below the caching threshold size,
>   (128 KB).
>
> Bug: chromium:719172
> Change-Id: Iaf7e2fa96e9bb441bda18b780ae41794af119900
> Reviewed-on: https://chromium-review.googlesource.com/c/1260209
> Commit-Queue: Bill Budge <bbudge@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Alexei Filippov <alph@chromium.org>
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Reviewed-by: Mythri Alle <mythria@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#626755}

Bug: chromium:719172
Change-Id: I168e1a4fba8ef49dd50406d67d2cf6833b20f64b
Reviewed-on: https://chromium-review.googlesource.com/c/1446559
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Mythri Alle <mythria@chromium.org>
Commit-Queue: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#627520}
[modify] https://crrev.com/7aca1c5411ca2106eb4793fa3a23beb89fd9d778/third_party/blink/renderer/bindings/core/v8/v8_wasm_response_extensions.cc
[modify] https://crrev.com/7aca1c5411ca2106eb4793fa3a23beb89fd9d778/third_party/blink/renderer/devtools/front_end/timeline/TimelineUIUtils.js
[modify] https://crrev.com/7aca1c5411ca2106eb4793fa3a23beb89fd9d778/third_party/blink/renderer/devtools/front_end/timeline_model/TimelineModel.js
[modify] https://crrev.com/7aca1c5411ca2106eb4793fa3a23beb89fd9d778/third_party/blink/renderer/platform/loader/fetch/resource_loader.cc
[modify] https://crrev.com/7aca1c5411ca2106eb4793fa3a23beb89fd9d778/third_party/blink/web_tests/SlowTests
[modify] https://crrev.com/7aca1c5411ca2106eb4793fa3a23beb89fd9d778/third_party/blink/web_tests/VirtualTestSuites
[add] https://crrev.com/7aca1c5411ca2106eb4793fa3a23beb89fd9d778/third_party/blink/web_tests/http/tests/devtools/wasm-isolated-code-cache/wasm-cache-test-expected.txt
[add] https://crrev.com/7aca1c5411ca2106eb4793fa3a23beb89fd9d778/third_party/blink/web_tests/http/tests/devtools/wasm-isolated-code-cache/wasm-cache-test.js
[add] https://crrev.com/7aca1c5411ca2106eb4793fa3a23beb89fd9d778/third_party/blink/web_tests/http/tests/wasm/resources/large.wasm
[modify] https://crrev.com/7aca1c5411ca2106eb4793fa3a23beb89fd9d778/third_party/blink/web_tests/http/tests/wasm/resources/load-wasm.php
[add] https://crrev.com/7aca1c5411ca2106eb4793fa3a23beb89fd9d778/third_party/blink/web_tests/http/tests/wasm/resources/small.wasm
[add] https://crrev.com/7aca1c5411ca2106eb4793fa3a23beb89fd9d778/third_party/blink/web_tests/http/tests/wasm/resources/wasm-cache-iframe.html
[add] https://crrev.com/7aca1c5411ca2106eb4793fa3a23beb89fd9d778/third_party/blink/web_tests/virtual/wasm-site-isolated-code-cache/http/tests/devtools/wasm-isolated-code-cache/README.txt
[add] https://crrev.com/7aca1c5411ca2106eb4793fa3a23beb89fd9d778/third_party/blink/web_tests/virtual/wasm-site-isolated-code-cache/http/tests/devtools/wasm-isolated-code-cache/wasm-cache-test-expected.txt

Comment 34 by bugdroid, Feb 5

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/20e95c29ca63625f4a39d2bc02d92315366550be

commit 20e95c29ca63625f4a39d2bc02d92315366550be
Author: Bill Budge <bbudge@chromium.org>
Date: Tue Feb 05 02:03:42 2019

[blink web_tests] Rework WebAssembly cache test

- Use async/await instead of promise chains.
- Properly start and stop Timeline, so events can propagate.
- Reenables test.

Bug:  chromium:927296 ,chromium:719172
Change-Id: I33f676e1993ca07380bc645606b422b45f4b090d
Reviewed-on: https://chromium-review.googlesource.com/c/1450616
Reviewed-by: Alexei Filippov <alph@chromium.org>
Commit-Queue: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628999}
[modify] https://crrev.com/20e95c29ca63625f4a39d2bc02d92315366550be/third_party/blink/web_tests/TestExpectations
[modify] https://crrev.com/20e95c29ca63625f4a39d2bc02d92315366550be/third_party/blink/web_tests/http/tests/devtools/wasm-isolated-code-cache/wasm-cache-test.js
[modify] https://crrev.com/20e95c29ca63625f4a39d2bc02d92315366550be/third_party/blink/web_tests/virtual/wasm-site-isolated-code-cache/http/tests/devtools/wasm-isolated-code-cache/wasm-cache-test-expected.txt

Comment 35 by bugdroid, Feb 6

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8ec43ccbd86814c4788d6c8afe86339eefe6cdea

commit 8ec43ccbd86814c4788d6c8afe86339eefe6cdea
Author: Bill Budge <bbudge@chromium.org>
Date: Wed Feb 06 20:25:54 2019

[WebAssembly] Start a field trial for Wasm compiled code caching

This CL depends on blink::features::kWasmCodeCache:
https://chromium-review.googlesource.com/c/chromium/src/+/1341071

...and hooking up Wasm's site isolated code cache instance:
https://chromium-review.googlesource.com/c/chromium/src/+/1260209

Both of these should land before this one.

Bug: chromium:719172
Change-Id: Ia721e931fb34c6ee3a4346f4b151ce316bc6501c
Reviewed-on: https://chromium-review.googlesource.com/c/1341076
Reviewed-by: Brian White <bcwhite@chromium.org>
Commit-Queue: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#629708}
[modify] https://crrev.com/8ec43ccbd86814c4788d6c8afe86339eefe6cdea/testing/variations/fieldtrial_testing_config.json

Comment 36 by bugdroid, Feb 8

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/237428d3c3e3f640b821272e17f1cc582380b668

commit 237428d3c3e3f640b821272e17f1cc582380b668
Author: Bill Budge <bbudge@chromium.org>
Date: Fri Feb 08 18:12:01 2019

[code cache] Detect entry write error and remove entry.

- Detects errors where entry write data call fails. In that
  case, deletes the entry.
- Detects bad entries while reading and signals them with an
  invalid response time.

Bug: chromium:719172
Change-Id: I3c977f9d2e2869c4dee6c83d45bc9d98cb9c0e93
Reviewed-on: https://chromium-review.googlesource.com/c/1459523
Reviewed-by: Mythri Alle <mythria@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Bill Budge <bbudge@chromium.org>
Cr-Commit-Position: refs/heads/master@{#630390}
[modify] https://crrev.com/237428d3c3e3f640b821272e17f1cc582380b668/content/browser/code_cache/generated_code_cache.cc
[modify] https://crrev.com/237428d3c3e3f640b821272e17f1cc582380b668/content/browser/code_cache/generated_code_cache.h
[modify] https://crrev.com/237428d3c3e3f640b821272e17f1cc582380b668/content/browser/code_cache/generated_code_cache_unittest.cc

Comment 37 by bugdroid, Feb 10

Project Member

Sign in to add a comment