New issue
Advanced search Search tips
Starred by 7 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Feature

Blocked on:
issue 912902
issue 837659
issue 860693
issue 883344
issue 901815
issue 905777
issue 907489
issue 909796
issue 920194

Blocking:
issue 924164



Sign in to add a comment
link

Issue 877044: Park Javascript source strings to reduce memory footprint

Reported by lizeb@chromium.org, Aug 23 Project Member

Issue description

Park and compress javascript source strings to reduce memory footprint.

Tracking bug.
 

Comment 1 by lizeb@chromium.org, Aug 23

Blockedon: 837659

Comment 2 by bugdroid1@chromium.org, Aug 27

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

commit aab50a3124b587eccdb1eb102ef1c9354a36e503
Author: Benoît Lizé <lizeb@chromium.org>
Date: Mon Aug 27 16:54:03 2018

Rename ShortExternalString to UncachedExternalString.

"short" external strings are not short, they mean that the external data
pointer is not cached. Rename the various classes and objects to align
with the actual meaning.

Bug: chromium:877044
Change-Id: Ie3d5baa9ad352ac6ca89f5ba1d066760825e4beb
Reviewed-on: https://chromium-review.googlesource.com/1185192
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Commit-Queue: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55432}
[modify] https://crrev.com/aab50a3124b587eccdb1eb102ef1c9354a36e503/src/bootstrapper.cc
[modify] https://crrev.com/aab50a3124b587eccdb1eb102ef1c9354a36e503/src/builtins/builtins-string-gen.cc
[modify] https://crrev.com/aab50a3124b587eccdb1eb102ef1c9354a36e503/src/code-stub-assembler.cc
[modify] https://crrev.com/aab50a3124b587eccdb1eb102ef1c9354a36e503/src/code-stub-assembler.h
[modify] https://crrev.com/aab50a3124b587eccdb1eb102ef1c9354a36e503/src/compiler/effect-control-linearizer.cc
[modify] https://crrev.com/aab50a3124b587eccdb1eb102ef1c9354a36e503/src/compiler/types.cc
[modify] https://crrev.com/aab50a3124b587eccdb1eb102ef1c9354a36e503/src/heap/factory.cc
[modify] https://crrev.com/aab50a3124b587eccdb1eb102ef1c9354a36e503/src/heap/setup-heap-internal.cc
[modify] https://crrev.com/aab50a3124b587eccdb1eb102ef1c9354a36e503/src/objects-definitions.h
[modify] https://crrev.com/aab50a3124b587eccdb1eb102ef1c9354a36e503/src/objects-printer.cc
[modify] https://crrev.com/aab50a3124b587eccdb1eb102ef1c9354a36e503/src/objects.cc
[modify] https://crrev.com/aab50a3124b587eccdb1eb102ef1c9354a36e503/src/objects.h
[modify] https://crrev.com/aab50a3124b587eccdb1eb102ef1c9354a36e503/src/objects/string-inl.h
[modify] https://crrev.com/aab50a3124b587eccdb1eb102ef1c9354a36e503/src/objects/string.h
[modify] https://crrev.com/aab50a3124b587eccdb1eb102ef1c9354a36e503/src/roots.h
[modify] https://crrev.com/aab50a3124b587eccdb1eb102ef1c9354a36e503/src/snapshot/deserializer.cc
[modify] https://crrev.com/aab50a3124b587eccdb1eb102ef1c9354a36e503/src/snapshot/serializer.cc
[modify] https://crrev.com/aab50a3124b587eccdb1eb102ef1c9354a36e503/test/cctest/test-regexp.cc
[modify] https://crrev.com/aab50a3124b587eccdb1eb102ef1c9354a36e503/tools/grokdump.py
[modify] https://crrev.com/aab50a3124b587eccdb1eb102ef1c9354a36e503/tools/heap-stats/categories.js
[modify] https://crrev.com/aab50a3124b587eccdb1eb102ef1c9354a36e503/tools/v8heapconst.py

Comment 3 by bugdroid1@chromium.org, Aug 30

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

commit e34e060cc63763310b7dfa1adac466463f11b3f8
Author: Benoit Lize <lizeb@chromium.org>
Date: Thu Aug 30 12:06:59 2018

blink: Move MovableString to platform/bindings.

A subsequent CL will rename MovableString to a more suitable name, this
was split to minimize the diff.

Also fix trivial "git cl lint" warnings.

Bug: 877044
Change-Id: I2b96530bc227100e59d840f5abeb5e7995521af9
Reviewed-on: https://chromium-review.googlesource.com/1193442
Commit-Queue: Benoit L <lizeb@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587526}
[modify] https://crrev.com/e34e060cc63763310b7dfa1adac466463f11b3f8/third_party/blink/renderer/bindings/core/v8/script_source_code.h
[modify] https://crrev.com/e34e060cc63763310b7dfa1adac466463f11b3f8/third_party/blink/renderer/core/loader/modulescript/document_module_script_fetcher.cc
[modify] https://crrev.com/e34e060cc63763310b7dfa1adac466463f11b3f8/third_party/blink/renderer/core/loader/modulescript/module_script_creation_params.h
[modify] https://crrev.com/e34e060cc63763310b7dfa1adac466463f11b3f8/third_party/blink/renderer/core/loader/resource/script_resource.h
[modify] https://crrev.com/e34e060cc63763310b7dfa1adac466463f11b3f8/third_party/blink/renderer/core/script/module_script.h
[modify] https://crrev.com/e34e060cc63763310b7dfa1adac466463f11b3f8/third_party/blink/renderer/core/script/script_loader.cc
[modify] https://crrev.com/e34e060cc63763310b7dfa1adac466463f11b3f8/third_party/blink/renderer/platform/BUILD.gn
[rename] https://crrev.com/e34e060cc63763310b7dfa1adac466463f11b3f8/third_party/blink/renderer/platform/bindings/movable_string.cc
[rename] https://crrev.com/e34e060cc63763310b7dfa1adac466463f11b3f8/third_party/blink/renderer/platform/bindings/movable_string.h
[rename] https://crrev.com/e34e060cc63763310b7dfa1adac466463f11b3f8/third_party/blink/renderer/platform/bindings/movable_string_test.cc
[modify] https://crrev.com/e34e060cc63763310b7dfa1adac466463f11b3f8/third_party/blink/renderer/platform/bindings/string_resource.h
[modify] https://crrev.com/e34e060cc63763310b7dfa1adac466463f11b3f8/third_party/blink/renderer/platform/bindings/v8_value_cache.h
[modify] https://crrev.com/e34e060cc63763310b7dfa1adac466463f11b3f8/third_party/blink/renderer/platform/loader/DEPS
[modify] https://crrev.com/e34e060cc63763310b7dfa1adac466463f11b3f8/third_party/blink/renderer/platform/loader/fetch/script_cached_metadata_handler.h
[modify] https://crrev.com/e34e060cc63763310b7dfa1adac466463f11b3f8/third_party/blink/renderer/platform/scheduler/DEPS
[modify] https://crrev.com/e34e060cc63763310b7dfa1adac466463f11b3f8/third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.cc
[modify] https://crrev.com/e34e060cc63763310b7dfa1adac466463f11b3f8/third_party/blink/renderer/platform/wtf/BUILD.gn
[modify] https://crrev.com/e34e060cc63763310b7dfa1adac466463f11b3f8/third_party/blink/renderer/platform/wtf/wtf_thread_data.cc
[modify] https://crrev.com/e34e060cc63763310b7dfa1adac466463f11b3f8/third_party/blink/renderer/platform/wtf/wtf_thread_data.h

Comment 4 by bugdroid1@chromium.org, Aug 30

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

commit 245b8ffc7a2ff951dff3b7ef61c35936611e659c
Author: Benoit Lize <lizeb@chromium.org>
Date: Thu Aug 30 12:11:09 2018

blink: Rename MovableString to ParkableString.

MovableStrings don't have anything to do with std::move(), change their
names to a more meaningful one.

Also fix a couple trivial "git cl lint" warnings.

Bug: 877044
Change-Id: I5a2b500c6161725bdb6b64b48af8554c2d6c260f
Reviewed-on: https://chromium-review.googlesource.com/1194024
Commit-Queue: Benoit L <lizeb@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587527}
[modify] https://crrev.com/245b8ffc7a2ff951dff3b7ef61c35936611e659c/third_party/blink/renderer/bindings/core/v8/script_source_code.cc
[modify] https://crrev.com/245b8ffc7a2ff951dff3b7ef61c35936611e659c/third_party/blink/renderer/bindings/core/v8/script_source_code.h
[modify] https://crrev.com/245b8ffc7a2ff951dff3b7ef61c35936611e659c/third_party/blink/renderer/core/loader/modulescript/document_module_script_fetcher.cc
[modify] https://crrev.com/245b8ffc7a2ff951dff3b7ef61c35936611e659c/third_party/blink/renderer/core/loader/modulescript/module_script_creation_params.h
[modify] https://crrev.com/245b8ffc7a2ff951dff3b7ef61c35936611e659c/third_party/blink/renderer/core/loader/resource/script_resource.cc
[modify] https://crrev.com/245b8ffc7a2ff951dff3b7ef61c35936611e659c/third_party/blink/renderer/core/loader/resource/script_resource.h
[modify] https://crrev.com/245b8ffc7a2ff951dff3b7ef61c35936611e659c/third_party/blink/renderer/core/script/module_map_test.cc
[modify] https://crrev.com/245b8ffc7a2ff951dff3b7ef61c35936611e659c/third_party/blink/renderer/core/script/module_script.cc
[modify] https://crrev.com/245b8ffc7a2ff951dff3b7ef61c35936611e659c/third_party/blink/renderer/core/script/module_script.h
[modify] https://crrev.com/245b8ffc7a2ff951dff3b7ef61c35936611e659c/third_party/blink/renderer/core/script/script_loader.cc
[modify] https://crrev.com/245b8ffc7a2ff951dff3b7ef61c35936611e659c/third_party/blink/renderer/platform/BUILD.gn
[delete] https://crrev.com/e34e060cc63763310b7dfa1adac466463f11b3f8/third_party/blink/renderer/platform/bindings/movable_string.cc
[delete] https://crrev.com/e34e060cc63763310b7dfa1adac466463f11b3f8/third_party/blink/renderer/platform/bindings/movable_string_test.cc
[add] https://crrev.com/245b8ffc7a2ff951dff3b7ef61c35936611e659c/third_party/blink/renderer/platform/bindings/parkable_string.cc
[rename] https://crrev.com/245b8ffc7a2ff951dff3b7ef61c35936611e659c/third_party/blink/renderer/platform/bindings/parkable_string.h
[add] https://crrev.com/245b8ffc7a2ff951dff3b7ef61c35936611e659c/third_party/blink/renderer/platform/bindings/parkable_string_test.cc
[modify] https://crrev.com/245b8ffc7a2ff951dff3b7ef61c35936611e659c/third_party/blink/renderer/platform/bindings/string_resource.h
[modify] https://crrev.com/245b8ffc7a2ff951dff3b7ef61c35936611e659c/third_party/blink/renderer/platform/bindings/v8_binding.h
[modify] https://crrev.com/245b8ffc7a2ff951dff3b7ef61c35936611e659c/third_party/blink/renderer/platform/bindings/v8_value_cache.cc
[modify] https://crrev.com/245b8ffc7a2ff951dff3b7ef61c35936611e659c/third_party/blink/renderer/platform/bindings/v8_value_cache.h
[modify] https://crrev.com/245b8ffc7a2ff951dff3b7ef61c35936611e659c/third_party/blink/renderer/platform/loader/DEPS
[modify] https://crrev.com/245b8ffc7a2ff951dff3b7ef61c35936611e659c/third_party/blink/renderer/platform/loader/fetch/script_cached_metadata_handler.h
[modify] https://crrev.com/245b8ffc7a2ff951dff3b7ef61c35936611e659c/third_party/blink/renderer/platform/scheduler/DEPS
[modify] https://crrev.com/245b8ffc7a2ff951dff3b7ef61c35936611e659c/third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.cc

Comment 5 by bugdroid1@chromium.org, Sep 4

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

commit a1da383fb3257e13912f4b10ea398d05bf221b56
Author: Benoît Lizé <lizeb@chromium.org>
Date: Tue Sep 04 14:09:04 2018

parsing: Lock ExternalStrings in the ExternalStringStream.

Utf*Characterstream caches the data pointer of ExternalStrings through
ExternalStringStream, so lock the strings in ExternalStringStream.

Bug: chromium:877044
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: I241caaf64e109b33e2f9982573e11c514410509c
Reviewed-on: https://chromium-review.googlesource.com/1194003
Commit-Queue: Benoit L <lizeb@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55613}
[modify] https://crrev.com/a1da383fb3257e13912f4b10ea398d05bf221b56/include/v8.h
[modify] https://crrev.com/a1da383fb3257e13912f4b10ea398d05bf221b56/src/heap/factory.cc
[modify] https://crrev.com/a1da383fb3257e13912f4b10ea398d05bf221b56/src/objects.cc
[modify] https://crrev.com/a1da383fb3257e13912f4b10ea398d05bf221b56/src/parsing/scanner-character-streams.cc
[modify] https://crrev.com/a1da383fb3257e13912f4b10ea398d05bf221b56/test/cctest/parsing/test-scanner-streams.cc
[modify] https://crrev.com/a1da383fb3257e13912f4b10ea398d05bf221b56/test/cctest/test-regexp.cc

Comment 6 by bugdroid1@chromium.org, Sep 4

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

commit 14a94ec56fbf0792c4f28adee97979645ef021ea
Author: Benoit Lize <lizeb@chromium.org>
Date: Tue Sep 04 07:41:47 2018

zlib/google: Make GzipCompress() take a StringPiece.

Bug: 877044
Change-Id: I193aec9eaf8e0f59413b4b0c58489aeac48b5ad2
Reviewed-on: https://chromium-review.googlesource.com/1202143
Commit-Queue: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588465}
[modify] https://crrev.com/14a94ec56fbf0792c4f28adee97979645ef021ea/third_party/zlib/google/compression_utils.cc
[modify] https://crrev.com/14a94ec56fbf0792c4f28adee97979645ef021ea/third_party/zlib/google/compression_utils.h

Comment 7 by bugdroid1@chromium.org, Sep 4

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

commit a1da383fb3257e13912f4b10ea398d05bf221b56
Author: Benoît Lizé <lizeb@chromium.org>
Date: Tue Sep 04 14:09:04 2018

parsing: Lock ExternalStrings in the ExternalStringStream.

Utf*Characterstream caches the data pointer of ExternalStrings through
ExternalStringStream, so lock the strings in ExternalStringStream.

Bug: chromium:877044
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: I241caaf64e109b33e2f9982573e11c514410509c
Reviewed-on: https://chromium-review.googlesource.com/1194003
Commit-Queue: Benoit L <lizeb@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55613}
[modify] https://crrev.com/a1da383fb3257e13912f4b10ea398d05bf221b56/include/v8.h
[modify] https://crrev.com/a1da383fb3257e13912f4b10ea398d05bf221b56/src/heap/factory.cc
[modify] https://crrev.com/a1da383fb3257e13912f4b10ea398d05bf221b56/src/objects.cc
[modify] https://crrev.com/a1da383fb3257e13912f4b10ea398d05bf221b56/src/parsing/scanner-character-streams.cc
[modify] https://crrev.com/a1da383fb3257e13912f4b10ea398d05bf221b56/test/cctest/parsing/test-scanner-streams.cc
[modify] https://crrev.com/a1da383fb3257e13912f4b10ea398d05bf221b56/test/cctest/test-regexp.cc

Comment 8 by bugdroid1@chromium.org, Sep 4

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

commit 14a94ec56fbf0792c4f28adee97979645ef021ea
Author: Benoit Lize <lizeb@chromium.org>
Date: Tue Sep 04 07:41:47 2018

zlib/google: Make GzipCompress() take a StringPiece.

Bug: 877044
Change-Id: I193aec9eaf8e0f59413b4b0c58489aeac48b5ad2
Reviewed-on: https://chromium-review.googlesource.com/1202143
Commit-Queue: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588465}
[modify] https://crrev.com/14a94ec56fbf0792c4f28adee97979645ef021ea/third_party/zlib/google/compression_utils.cc
[modify] https://crrev.com/14a94ec56fbf0792c4f28adee97979645ef021ea/third_party/zlib/google/compression_utils.h

Comment 9 by bugdroid1@chromium.org, Sep 4

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

commit a1da383fb3257e13912f4b10ea398d05bf221b56
Author: Benoît Lizé <lizeb@chromium.org>
Date: Tue Sep 04 14:09:04 2018

parsing: Lock ExternalStrings in the ExternalStringStream.

Utf*Characterstream caches the data pointer of ExternalStrings through
ExternalStringStream, so lock the strings in ExternalStringStream.

Bug: chromium:877044
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: I241caaf64e109b33e2f9982573e11c514410509c
Reviewed-on: https://chromium-review.googlesource.com/1194003
Commit-Queue: Benoit L <lizeb@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55613}
[modify] https://crrev.com/a1da383fb3257e13912f4b10ea398d05bf221b56/include/v8.h
[modify] https://crrev.com/a1da383fb3257e13912f4b10ea398d05bf221b56/src/heap/factory.cc
[modify] https://crrev.com/a1da383fb3257e13912f4b10ea398d05bf221b56/src/objects.cc
[modify] https://crrev.com/a1da383fb3257e13912f4b10ea398d05bf221b56/src/parsing/scanner-character-streams.cc
[modify] https://crrev.com/a1da383fb3257e13912f4b10ea398d05bf221b56/test/cctest/parsing/test-scanner-streams.cc
[modify] https://crrev.com/a1da383fb3257e13912f4b10ea398d05bf221b56/test/cctest/test-regexp.cc

Comment 10 by bugdroid1@chromium.org, Sep 4

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

commit 14a94ec56fbf0792c4f28adee97979645ef021ea
Author: Benoit Lize <lizeb@chromium.org>
Date: Tue Sep 04 07:41:47 2018

zlib/google: Make GzipCompress() take a StringPiece.

Bug: 877044
Change-Id: I193aec9eaf8e0f59413b4b0c58489aeac48b5ad2
Reviewed-on: https://chromium-review.googlesource.com/1202143
Commit-Queue: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588465}
[modify] https://crrev.com/14a94ec56fbf0792c4f28adee97979645ef021ea/third_party/zlib/google/compression_utils.cc
[modify] https://crrev.com/14a94ec56fbf0792c4f28adee97979645ef021ea/third_party/zlib/google/compression_utils.h

Comment 11 by bugdroid1@chromium.org, Sep 5

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

commit 5a1630f17715a9612c9d5f6aeb5e5f8507276502
Author: Benoit Lize <lizeb@chromium.org>
Date: Wed Sep 05 12:50:44 2018

blink: Add ParkableStringManager, don't track off-main thread strings.

ParkableStringManager tracks all the current ParkableStrings. This
replace ParkableStringTable.
Since strings not on the main thread were never parked, don't remember
them at all.

Bug: 877044
Change-Id: I17ad0b4fc38264ddb344776a5b202fa5475bb906
Reviewed-on: https://chromium-review.googlesource.com/1199383
Commit-Queue: Benoit L <lizeb@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588843}
[modify] https://crrev.com/5a1630f17715a9612c9d5f6aeb5e5f8507276502/third_party/blink/renderer/platform/BUILD.gn
[modify] https://crrev.com/5a1630f17715a9612c9d5f6aeb5e5f8507276502/third_party/blink/renderer/platform/bindings/parkable_string.cc
[modify] https://crrev.com/5a1630f17715a9612c9d5f6aeb5e5f8507276502/third_party/blink/renderer/platform/bindings/parkable_string.h
[add] https://crrev.com/5a1630f17715a9612c9d5f6aeb5e5f8507276502/third_party/blink/renderer/platform/bindings/parkable_string_manager.cc
[add] https://crrev.com/5a1630f17715a9612c9d5f6aeb5e5f8507276502/third_party/blink/renderer/platform/bindings/parkable_string_manager.h
[modify] https://crrev.com/5a1630f17715a9612c9d5f6aeb5e5f8507276502/third_party/blink/renderer/platform/bindings/parkable_string_test.cc
[modify] https://crrev.com/5a1630f17715a9612c9d5f6aeb5e5f8507276502/third_party/blink/renderer/platform/scheduler/DEPS
[modify] https://crrev.com/5a1630f17715a9612c9d5f6aeb5e5f8507276502/third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.cc

Comment 12 by bugdroid1@chromium.org, Sep 6

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

commit 117107a4df9c4a066e3af0f6d8e406cd6af78a9c
Author: Benoit Lize <lizeb@chromium.org>
Date: Thu Sep 06 15:33:06 2018

blink: Add support for locking ParkableString.

ExternalStringResource can be locked in V8, meaning that it should not
be parked. Add support in Chrome for this.
Separately, some blink strings will never be parked. For these, allow V8 to
cache the data pointer.

Bug: 877044
Change-Id: I0100a3b13887c417dfcf568186c9cf7f9c57706f
Reviewed-on: https://chromium-review.googlesource.com/1203376
Commit-Queue: Benoit L <lizeb@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589176}
[modify] https://crrev.com/117107a4df9c4a066e3af0f6d8e406cd6af78a9c/third_party/blink/renderer/platform/bindings/parkable_string.cc
[modify] https://crrev.com/117107a4df9c4a066e3af0f6d8e406cd6af78a9c/third_party/blink/renderer/platform/bindings/parkable_string.h
[modify] https://crrev.com/117107a4df9c4a066e3af0f6d8e406cd6af78a9c/third_party/blink/renderer/platform/bindings/parkable_string_manager.cc
[modify] https://crrev.com/117107a4df9c4a066e3af0f6d8e406cd6af78a9c/third_party/blink/renderer/platform/bindings/parkable_string_test.cc
[modify] https://crrev.com/117107a4df9c4a066e3af0f6d8e406cd6af78a9c/third_party/blink/renderer/platform/bindings/string_resource.h

Comment 13 by bugdroid1@chromium.org, Sep 7

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

commit 923d919ccddc5de9a7e42d96d7035e1c025d0c00
Author: Benoit Lize <lizeb@chromium.org>
Date: Fri Sep 07 13:41:58 2018

blink: Enforce ParkableString thread restrictions when DCHECK_IS_ON().

Bug: 877044
Change-Id: I6b3e7be16ade21bd9c511a6a4fda10930575efa4
Reviewed-on: https://chromium-review.googlesource.com/1210782
Commit-Queue: Benoit L <lizeb@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589505}
[modify] https://crrev.com/923d919ccddc5de9a7e42d96d7035e1c025d0c00/third_party/blink/renderer/platform/bindings/parkable_string.cc
[modify] https://crrev.com/923d919ccddc5de9a7e42d96d7035e1c025d0c00/third_party/blink/renderer/platform/bindings/parkable_string.h

Comment 14 by bugdroid1@chromium.org, Sep 10

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

commit 3270b5b3dad92c048aa3b929df1e9183ab9f4da2
Author: Benoit Lize <lizeb@chromium.org>
Date: Mon Sep 10 11:49:39 2018

blink/bindings: Simplify null ParkableString.

Currently, we have:
ParkableString -> ParkableStringImpl -> Null String
This removes the indirections, saving memory and making code simpler.

Bug: 877044
Change-Id: Ib6d22cad836c1eca9918e0198f5e6a1af8fccbe2
Reviewed-on: https://chromium-review.googlesource.com/1213262
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589888}
[modify] https://crrev.com/3270b5b3dad92c048aa3b929df1e9183ab9f4da2/third_party/blink/renderer/platform/bindings/parkable_string.cc
[modify] https://crrev.com/3270b5b3dad92c048aa3b929df1e9183ab9f4da2/third_party/blink/renderer/platform/bindings/parkable_string.h
[modify] https://crrev.com/3270b5b3dad92c048aa3b929df1e9183ab9f4da2/third_party/blink/renderer/platform/bindings/parkable_string_manager.cc
[modify] https://crrev.com/3270b5b3dad92c048aa3b929df1e9183ab9f4da2/third_party/blink/renderer/platform/bindings/parkable_string_manager.h
[modify] https://crrev.com/3270b5b3dad92c048aa3b929df1e9183ab9f4da2/third_party/blink/renderer/platform/bindings/parkable_string_test.cc
[modify] https://crrev.com/3270b5b3dad92c048aa3b929df1e9183ab9f4da2/third_party/blink/renderer/platform/bindings/string_resource.cc
[modify] https://crrev.com/3270b5b3dad92c048aa3b929df1e9183ab9f4da2/third_party/blink/renderer/platform/bindings/string_resource.h

Comment 15 by bugdroid1@chromium.org, Sep 11

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

commit 88924f039d0b96626c33c2183bd177952b00a2a6
Author: Benoit Lize <lizeb@chromium.org>
Date: Tue Sep 11 09:25:24 2018

blink/bindings: ASAN checks to ensure proper ParkableString accesses.

A ParkableString underlying string memory should not be cached.
Following a suggestion by pasko@, add ASAN instrumentation to make sure
this is actually the case.

Bug: 877044
Change-Id: I463ad229b118f2e629b460f1a2beed41fc54b6a7
Reviewed-on: https://chromium-review.googlesource.com/1216422
Reviewed-by: Egor Pasko <pasko@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590251}
[modify] https://crrev.com/88924f039d0b96626c33c2183bd177952b00a2a6/third_party/blink/renderer/platform/bindings/parkable_string.cc

Comment 16 by bugdroid1@chromium.org, Sep 11

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

commit a1153d79fe572bb3797c69d963ad0de62470e675
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Tue Sep 11 10:13:12 2018

Revert "blink/bindings: ASAN checks to ensure proper ParkableString accesses."

This reverts commit 88924f039d0b96626c33c2183bd177952b00a2a6.

Reason for revert:

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

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20ASAN/16046

Sample Failed Step: compile

Original change's description:
> blink/bindings: ASAN checks to ensure proper ParkableString accesses.
> 
> A ParkableString underlying string memory should not be cached.
> Following a suggestion by pasko@, add ASAN instrumentation to make sure
> this is actually the case.
> 
> Bug: 877044
> Change-Id: I463ad229b118f2e629b460f1a2beed41fc54b6a7
> Reviewed-on: https://chromium-review.googlesource.com/1216422
> Reviewed-by: Egor Pasko <pasko@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Commit-Queue: Benoit L <lizeb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#590251}

Change-Id: I3ce53830604e42a6bef13da80a420c44b9d5df11
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 877044
Reviewed-on: https://chromium-review.googlesource.com/1218548
Cr-Commit-Position: refs/heads/master@{#590254}
[modify] https://crrev.com/a1153d79fe572bb3797c69d963ad0de62470e675/third_party/blink/renderer/platform/bindings/parkable_string.cc

Comment 17 by bugdroid1@chromium.org, Sep 11

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

commit 1ec5add49bbd0dbd4a15c0f1d675fec151aad57a
Author: Benoit Lize <lizeb@chromium.org>
Date: Tue Sep 11 12:36:17 2018

Reland "blink/bindings: ASAN checks to ensure proper ParkableString accesses."

This is a reland of 88924f039d0b96626c33c2183bd177952b00a2a6

Added a missing DCHECK_IS_ON() check.

Original change's description:
> blink/bindings: ASAN checks to ensure proper ParkableString accesses.
>
> A ParkableString underlying string memory should not be cached.
> Following a suggestion by pasko@, add ASAN instrumentation to make sure
> this is actually the case.
>
> Bug: 877044
> Change-Id: I463ad229b118f2e629b460f1a2beed41fc54b6a7
> Reviewed-on: https://chromium-review.googlesource.com/1216422
> Reviewed-by: Egor Pasko <pasko@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Commit-Queue: Benoit L <lizeb@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#590251}

Bug: 877044
Change-Id: I777d53528037015ff3d16cd8c5ce2b649788848b
Reviewed-on: https://chromium-review.googlesource.com/1219486
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Egor Pasko <pasko@chromium.org>
Commit-Queue: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590276}
[modify] https://crrev.com/1ec5add49bbd0dbd4a15c0f1d675fec151aad57a/third_party/blink/renderer/platform/bindings/parkable_string.cc

Comment 18 by lizeb@chromium.org, Sep 14

Blockedon: 883344

Comment 19 by bugdroid1@chromium.org, Oct 22

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

commit 9aa76aede80b2e9317f02dccd00010ea2967d2c8
Author: Benoit Lize <lizeb@chromium.org>
Date: Mon Oct 22 10:55:24 2018

blink/bindings: Fix false-positive ASAN check.

A ParkableString underlying String may be atomic. In this case, as long
as it it alive, there is a raw pointer reference to it in a per-thread
table. This can lead to a use-after-poison as the string gets poisoned
whereas it is still in the table.

This is due to not freeing string_ in ParkableStringImpl. To fix that,
don't poison AtomicStrings (which are not the majority of
ParkableString).

This is a false positive as when real parking happens the underlying
string would be freed, hence removed from the AtomicStringTable.

Bug:  883344 ,877044
Change-Id: I685260eafe31da4cafed150b74870a08aa61ed40
Reviewed-on: https://chromium-review.googlesource.com/c/1228057
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601522}
[modify] https://crrev.com/9aa76aede80b2e9317f02dccd00010ea2967d2c8/third_party/blink/renderer/platform/bindings/parkable_string.cc

Comment 20 by bugdroid1@chromium.org, Oct 23

Project Member

Comment 21 by bugdroid1@chromium.org, Oct 24

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

commit 47546c89ff7d5fd72d60da3e73e2a62258b5985b
Author: Benoît Lizé <lizeb@chromium.org>
Date: Wed Oct 24 15:59:26 2018

blink/bindings: Park ParkableStrings in a separate task.

When a ParkableString is parked, do it in a separate task on the requester
thread. This is required to introduce compression.

This asynchronous parking means that a string can be in 3 states: unparked,
parking in progress or parked.

This CL adds:
- Asynchronous parking.
- Tracking of parked strings in ParkableStringManager
- Actual string parking with DCHECK_IS_ON()

Bug: 877044
Change-Id: Iece0e1338872aa6c315c9417a1050107494b676c
Reviewed-on: https://chromium-review.googlesource.com/c/1293570
Commit-Queue: Benoit L <lizeb@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602353}
[modify] https://crrev.com/47546c89ff7d5fd72d60da3e73e2a62258b5985b/third_party/blink/renderer/platform/bindings/parkable_string.cc
[modify] https://crrev.com/47546c89ff7d5fd72d60da3e73e2a62258b5985b/third_party/blink/renderer/platform/bindings/parkable_string.h
[modify] https://crrev.com/47546c89ff7d5fd72d60da3e73e2a62258b5985b/third_party/blink/renderer/platform/bindings/parkable_string_manager.cc
[modify] https://crrev.com/47546c89ff7d5fd72d60da3e73e2a62258b5985b/third_party/blink/renderer/platform/bindings/parkable_string_manager.h
[modify] https://crrev.com/47546c89ff7d5fd72d60da3e73e2a62258b5985b/third_party/blink/renderer/platform/bindings/parkable_string_test.cc
[modify] https://crrev.com/47546c89ff7d5fd72d60da3e73e2a62258b5985b/third_party/blink/renderer/platform/bindings/string_resource.h

Comment 22 by bugdroid1@chromium.org, Oct 30

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

commit 370d3c5b036bdc5e9cf1513a1316e6b128228fa6
Author: Benoît Lizé <lizeb@chromium.org>
Date: Tue Oct 30 11:59:24 2018

blink/bindings: Compress ParkableStrings in the background.

Adds actual zlib compression on a background thread, and decompression in
foreground.

Bug: 877044
Change-Id: I62a4c4fc05071bb3ce67df91b4bb9f53e34e2d85
Reviewed-on: https://chromium-review.googlesource.com/c/1301598
Commit-Queue: Benoit L <lizeb@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603863}
[modify] https://crrev.com/370d3c5b036bdc5e9cf1513a1316e6b128228fa6/third_party/blink/renderer/platform/bindings/DEPS
[modify] https://crrev.com/370d3c5b036bdc5e9cf1513a1316e6b128228fa6/third_party/blink/renderer/platform/bindings/parkable_string.cc
[modify] https://crrev.com/370d3c5b036bdc5e9cf1513a1316e6b128228fa6/third_party/blink/renderer/platform/bindings/parkable_string.h
[modify] https://crrev.com/370d3c5b036bdc5e9cf1513a1316e6b128228fa6/third_party/blink/renderer/platform/bindings/parkable_string_test.cc

Comment 23 by bugdroid1@chromium.org, Nov 5

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

commit ed230ab9e6b5998d5eb107ab43e3d150da45678f
Author: Benoît Lizé <lizeb@chromium.org>
Date: Mon Nov 05 11:51:43 2018

blink/bindings: Add (de)compression histograms for ParkableString.

Records the size, latency and throughput of parkable strings at compression and
decompression time. As these values are correlated, recording separate
histograms is necessary, and as not all compressed strings are decompressed,
separate histograms for compression and decompression are required.

Bug: 877044
Change-Id: Iec53e6d4f249e109165b0b3be6e2e245fc2bfd40
Reviewed-on: https://chromium-review.googlesource.com/c/1307440
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605296}
[modify] https://crrev.com/ed230ab9e6b5998d5eb107ab43e3d150da45678f/third_party/blink/renderer/platform/bindings/parkable_string.cc
[modify] https://crrev.com/ed230ab9e6b5998d5eb107ab43e3d150da45678f/third_party/blink/renderer/platform/bindings/parkable_string_test.cc
[modify] https://crrev.com/ed230ab9e6b5998d5eb107ab43e3d150da45678f/tools/metrics/histograms/histograms.xml

Comment 24 by lizeb@chromium.org, Nov 5

Blockedon: 901815

Comment 25 by bugdroid1@chromium.org, Nov 9

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

commit 43d0eed3db77a5e6167b5ebbf6ecb3710790272a
Author: Benoît Lizé <lizeb@chromium.org>
Date: Fri Nov 09 11:53:38 2018

blink/bindings: Record compressed size and savings for ParkableString.

Records the total size of parkable strings, the total compressed size, the
average compression ratio and the total savings from compression. As these
values are correlated, separate histograms are required.

The recording happens 30s after compression tasks are kicked off, provided that
the renderer's state (foreground/background) hasn't changed in the meantime, to
help with the interpretation of results. 30s is chosen as it's long enough for
compression to be done, and short enough to minimize the risk of the renderer
getting killed and/or moving back to foreground.

Also remove an outdated comment in parkable_string.cc.

Bug: 877044
Change-Id: Ic8cf4a410d182387ffd1d37b23d84b5426d227cb
Reviewed-on: https://chromium-review.googlesource.com/c/1323549
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Commit-Queue: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606800}
[modify] https://crrev.com/43d0eed3db77a5e6167b5ebbf6ecb3710790272a/third_party/blink/renderer/platform/bindings/parkable_string.h
[modify] https://crrev.com/43d0eed3db77a5e6167b5ebbf6ecb3710790272a/third_party/blink/renderer/platform/bindings/parkable_string_manager.cc
[modify] https://crrev.com/43d0eed3db77a5e6167b5ebbf6ecb3710790272a/third_party/blink/renderer/platform/bindings/parkable_string_manager.h
[modify] https://crrev.com/43d0eed3db77a5e6167b5ebbf6ecb3710790272a/third_party/blink/renderer/platform/bindings/parkable_string_test.cc
[modify] https://crrev.com/43d0eed3db77a5e6167b5ebbf6ecb3710790272a/tools/metrics/histograms/histograms.xml

Comment 26 by bugdroid1@chromium.org, Nov 12

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

commit ba80b64385e5b528583eadcd9b44c6c3869a9239
Author: Benoît Lizé <lizeb@chromium.org>
Date: Mon Nov 12 16:49:31 2018

blink/bindings: Actually record statistics for ParkableString.

Fix typo in https://chromium-review.googlesource.com/c/chromium/src/+/1323549

Bug: 877044
Change-Id: I377da8d819b3abbe27df8245e608e55c49759f05
Reviewed-on: https://chromium-review.googlesource.com/c/1329799
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607261}
[modify] https://crrev.com/ba80b64385e5b528583eadcd9b44c6c3869a9239/third_party/blink/renderer/platform/bindings/parkable_string_manager.cc

Comment 27 by bugdroid1@chromium.org, Nov 13

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

commit a852678d14edee58ec1f83c56196547a4a5cc09b
Author: Benoît Lizé <lizeb@chromium.org>
Date: Tue Nov 13 08:07:02 2018

blink/bindings: Improve ParkableStringManager testing.

|ParkableStringManager::SetRendererBackgrounded()| schedules delayed tasks, but
tests were not waiting for the tasks to run, instead directly calling the
delayed functions.

This adds a test checking for proper task sequencing, and changes other tests to
properly wait for tasks. As a consequence, the ParkableStringManager tests no
longer need to be friend with ParkableStringManager, provided that |Size()|
becomes public.

Also fixes a couple "git cl lint" warnings.

Bug: 877044
Change-Id: I15ea4285596dc744969e0bce63c5687e6fed023c
Reviewed-on: https://chromium-review.googlesource.com/c/1331476
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607518}
[modify] https://crrev.com/a852678d14edee58ec1f83c56196547a4a5cc09b/third_party/blink/renderer/platform/bindings/parkable_string.cc
[modify] https://crrev.com/a852678d14edee58ec1f83c56196547a4a5cc09b/third_party/blink/renderer/platform/bindings/parkable_string_manager.cc
[modify] https://crrev.com/a852678d14edee58ec1f83c56196547a4a5cc09b/third_party/blink/renderer/platform/bindings/parkable_string_manager.h
[modify] https://crrev.com/a852678d14edee58ec1f83c56196547a4a5cc09b/third_party/blink/renderer/platform/bindings/parkable_string_test.cc

Comment 28 by bugdroid1@chromium.org, Nov 15

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

commit 3cb93d0aa21a7cb92b92ca169104e5efab7c295c
Author: Benoît Lizé <lizeb@chromium.org>
Date: Thu Nov 15 07:38:57 2018

blink/bindings: Fix false-positive ASAN warning in ParkableString.

The following sequence is racy with the current ASAN checks in ParkableString,
on the main thread:

Park()
Lock()
ToString()
Unlock()

Park() poisons the string, ToString() unpoisons it, and Unlock() poisons it
again. If this last call happens while the compression is in progress, then this
is a use-after-poison.

This is not a real issue, merely an overaly eager poisoning, still making using
ASAN builds painful. Fix it by making sure the string stays unpoisoned during
compression.
Also adds a regression test.

Bug:  905137 ,877044
Change-Id: I5276b9ae6eee4abe2f2bf041818d1ba17358a80a
Reviewed-on: https://chromium-review.googlesource.com/c/1335585
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608289}
[modify] https://crrev.com/3cb93d0aa21a7cb92b92ca169104e5efab7c295c/third_party/blink/renderer/platform/bindings/parkable_string.cc
[modify] https://crrev.com/3cb93d0aa21a7cb92b92ca169104e5efab7c295c/third_party/blink/renderer/platform/bindings/parkable_string_test.cc

Comment 29 by bugdroid1@chromium.org, Nov 21

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

commit 8e8c6250ee1cf99df021a41d9a2dc8f49e947722
Author: Benoît Lizé <lizeb@chromium.org>
Date: Wed Nov 21 13:14:20 2018

blink/bindings: Keep compressed ParkableStrings when unparking, and reuse it.

When a ParkableString is unparked, keep the compressed data around, allowing to
drop the uncompressed data at no cost later. As decompression is much (>10x)
cheaper than compression and compressed data smaller, this is usually a good
tradeoff. This also allows to recover memory when a string is unparked once, and
its content is not touched again soon.

Concretely, we introduce:
- "synchronous parking" when parking is merely dropping the decompressed
  representation.
- "synchronous only" string parking in ParkableStringManager
- Drop all parkable strings when receiving a OnPurgeMemory() notification.

Also, improve and simplify testing, with fewer "friend" function and tighter
compression test.

Bug: 877044
Change-Id: I4b66ca5ddc67b2a6e49bfd1790e1788870642b04
Reviewed-on: https://chromium-review.googlesource.com/c/1329975
Reviewed-by: Benoit L <lizeb@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610024}
[modify] https://crrev.com/8e8c6250ee1cf99df021a41d9a2dc8f49e947722/third_party/blink/renderer/platform/bindings/DEPS
[modify] https://crrev.com/8e8c6250ee1cf99df021a41d9a2dc8f49e947722/third_party/blink/renderer/platform/bindings/parkable_string.cc
[modify] https://crrev.com/8e8c6250ee1cf99df021a41d9a2dc8f49e947722/third_party/blink/renderer/platform/bindings/parkable_string.h
[modify] https://crrev.com/8e8c6250ee1cf99df021a41d9a2dc8f49e947722/third_party/blink/renderer/platform/bindings/parkable_string_manager.cc
[modify] https://crrev.com/8e8c6250ee1cf99df021a41d9a2dc8f49e947722/third_party/blink/renderer/platform/bindings/parkable_string_manager.h
[modify] https://crrev.com/8e8c6250ee1cf99df021a41d9a2dc8f49e947722/third_party/blink/renderer/platform/bindings/parkable_string_test.cc

Comment 30 by lizeb@chromium.org, Nov 21

Blockedon: 905777

Comment 31 by bugdroid1@chromium.org, Nov 26

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

commit 9c4521577a2e110e92c4f641b10287ae315d19af
Author: Benoît Lizé <lizeb@chromium.org>
Date: Mon Nov 26 13:24:10 2018

blink/bindings: Don't post empty tasks when compression is not enabled.

Don't post delayed tasks that would (1) do nothing
(ParkAllIfrendererBackgrounded) and (2) record meaningless statistics
(DropStringsWithCompressedDataAndRecordStatistics()) when the feature is not
enabled.

This is not an issue to interpret finch trial results, just creates unnecessary
work.

Bug: 877044
Change-Id: I39ed2f787b171334d4e8fc7822599894cd9a4310
Reviewed-on: https://chromium-review.googlesource.com/c/1349650
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610835}
[modify] https://crrev.com/9c4521577a2e110e92c4f641b10287ae315d19af/third_party/blink/renderer/platform/bindings/parkable_string_manager.cc

Comment 32 by cavalcantii@chromium.org, Nov 29

Cc: cavalcantii@chromium.org

Comment 33 by lizeb@chromium.org, Dec 3

Blockedon: 907489

Comment 34 by lizeb@chromium.org, Dec 3

Blockedon: 909796

Comment 35 by bugdroid1@chromium.org, Dec 4

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

commit 9c64616cc5fdb5a9328a7b9e0f439b03ebc2ffdd
Author: Benoît Lizé <lizeb@chromium.org>
Date: Tue Dec 04 11:26:05 2018

blink/bindings: Handle foreground OnPurgeMemory() signals.

OnPurgeMemory() happens in case of memory pressure, and also when a renderer has
been backgrounded for some time. If received while in foreground, then we should
reduce memory footprint as much as possible.

To that end:
- Park all parkable strings
- Drop all compressed representation from strings that cannot be parked.

Bug: 877044
Change-Id: I6119fbf64f4954a9abe3b8466f7e7cbdff807e8f
Reviewed-on: https://chromium-review.googlesource.com/c/1348061
Commit-Queue: Benoit L <lizeb@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613509}
[modify] https://crrev.com/9c64616cc5fdb5a9328a7b9e0f439b03ebc2ffdd/third_party/blink/renderer/platform/bindings/parkable_string.cc
[modify] https://crrev.com/9c64616cc5fdb5a9328a7b9e0f439b03ebc2ffdd/third_party/blink/renderer/platform/bindings/parkable_string.h
[modify] https://crrev.com/9c64616cc5fdb5a9328a7b9e0f439b03ebc2ffdd/third_party/blink/renderer/platform/bindings/parkable_string_manager.cc
[modify] https://crrev.com/9c64616cc5fdb5a9328a7b9e0f439b03ebc2ffdd/third_party/blink/renderer/platform/bindings/parkable_string_manager.h
[modify] https://crrev.com/9c64616cc5fdb5a9328a7b9e0f439b03ebc2ffdd/third_party/blink/renderer/platform/bindings/parkable_string_test.cc

Comment 36 by lizeb@chromium.org, Dec 7

Blockedon: 912902

Comment 37 by lizeb@chromium.org, Jan 9

Blockedon: 920194

Comment 38 by bugdroid1@chromium.org, Jan 21

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

commit a37d56fb71b13f93cd25dffc786c5916f65d385d
Author: Benoît Lizé <lizeb@chromium.org>
Date: Mon Jan 21 16:35:14 2019

blink/bindings: Add memory-infra dump provider to ParkableStringManager.

Bug: 877044
Change-Id: I8bff95c4853e0ec19ddc2523191288d82a9cff90
Reviewed-on: https://chromium-review.googlesource.com/c/1422439
Commit-Queue: Benoit L <lizeb@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624603}
[modify] https://crrev.com/a37d56fb71b13f93cd25dffc786c5916f65d385d/third_party/blink/renderer/core/loader/resource/script_resource.cc
[modify] https://crrev.com/a37d56fb71b13f93cd25dffc786c5916f65d385d/third_party/blink/renderer/platform/bindings/parkable_string.cc
[modify] https://crrev.com/a37d56fb71b13f93cd25dffc786c5916f65d385d/third_party/blink/renderer/platform/bindings/parkable_string.h
[modify] https://crrev.com/a37d56fb71b13f93cd25dffc786c5916f65d385d/third_party/blink/renderer/platform/bindings/parkable_string_manager.cc
[modify] https://crrev.com/a37d56fb71b13f93cd25dffc786c5916f65d385d/third_party/blink/renderer/platform/bindings/parkable_string_manager.h
[modify] https://crrev.com/a37d56fb71b13f93cd25dffc786c5916f65d385d/third_party/blink/renderer/platform/bindings/parkable_string_test.cc
[modify] https://crrev.com/a37d56fb71b13f93cd25dffc786c5916f65d385d/third_party/blink/renderer/platform/exported/platform.cc

Comment 39 by lizeb@chromium.org, Jan 22

Blocking: 924164

Comment 40 by bugdroid, Feb 7

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

commit 326b13478fb2e253f6442ee5316d2f13c8a895ba
Author: Benoît Lizé <lizeb@chromium.org>
Date: Thu Feb 07 14:58:32 2019

blink/bindings: Make foreground and background compression mutually exclusive.

Cleans up feature flags for string parking, making foreground and background
compression mutually exclusive. Compression on memory pressure is kept in both
cases.

This does not remove background compression, as the foreground one is still
experimental, whereas the background one has been validated by deb/beta/stable
experiments.

Bug: 877044, 924164
Change-Id: I79b65e6d0073b18a704055e0cfeb54ba451e20c2
Reviewed-on: https://chromium-review.googlesource.com/c/1458200
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#629965}
[modify] https://crrev.com/326b13478fb2e253f6442ee5316d2f13c8a895ba/third_party/blink/renderer/platform/bindings/parkable_string_manager.cc
[modify] https://crrev.com/326b13478fb2e253f6442ee5316d2f13c8a895ba/third_party/blink/renderer/platform/bindings/parkable_string_test.cc

Comment 41 by bugdroid, Feb 8

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

commit d884033f8e2cfdc285f921bea7a19f1437d1cfb4
Author: Benoît Lizé <lizeb@chromium.org>
Date: Fri Feb 08 16:25:38 2019

blink/bindings: Enable background ParkableString compression by default.

Enabling a feature on trunk is required to ramp up the feature in stable.
The feature was already enabled on bots, so this CL removes the bots
configuration, but no behavior change is expected on bots.

Bug: 877044
Change-Id: I6ce5c45934fe34e4e22dab4b916058297b86a217
Reviewed-on: https://chromium-review.googlesource.com/c/1458201
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Commit-Queue: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#630339}
[modify] https://crrev.com/d884033f8e2cfdc285f921bea7a19f1437d1cfb4/testing/variations/fieldtrial_testing_config.json
[modify] https://crrev.com/d884033f8e2cfdc285f921bea7a19f1437d1cfb4/third_party/blink/renderer/platform/bindings/parkable_string_manager.h
[modify] https://crrev.com/d884033f8e2cfdc285f921bea7a19f1437d1cfb4/third_party/blink/renderer/platform/bindings/parkable_string_test.cc

Sign in to add a comment