commit 4a221ace5b60593f5399670c98f3f8ffd2b49872
author Benoît Lizé <lizeb@chromium.org> Mon Mar 09 13:42:19 2020
committer Commit Bot <commit-bot@chromium.org> Mon Mar 09 13:42:19 2020
tree 7377c934adf102bef8760b5316dd746974007af0
parent 72b79d5cda8ad9e95fabb5fdc551361216491067

blink/bindings: Reduce the overhead of ParkableString.

The overhead of ParkableString is:
- sizeof(ParkableString)
- sizeof(ParkableStringImpl) + digest + compressed data (if compressed)

The first one is small (sizeof(scoped_refptr<>), which is
sizeof(void*)), but the second one is non-trivial. This is due to:
- Need to cache metadata about a string when it is parked, for instance
  Is8Bit(), or its initial length.
- SHA256 hash used to deduplicate parked and unparked strings
- WTF::Mutex, which includes a pthread_mutex_t, which is large on most
  platforms.
- Handling of recursive locking, plus ParkableStringImpl being a
  refcounted class itself.

This is problematic, since it means that callers must know whether the
string is large, and either use WTF::String for small ones, or
ParkableString for large ones. It leads to more complex and duplicated
code, or to potential memory regressions.

As ParkableString is starting to be used in more places (e.g. for
trusted scripts), having to handle this becomes cumbersome and more
error-prone. On the other hand, all the metadata outlined above is not
necessary if the string is never parked, which is the case for small
ones. So the desired state is: short ParkableStrings are cheap enough so
that they can be used no matter the string length.

There are two simple ways to handle this:
- Have ParkableString hold the StringImpl directly for short strings
- Separate the metadata in ParkableStringImpl.

The first approach seems simpler, but would introduce non trivial
complexity in V8's value cache for strings (since it caches a
ref-counted impl for strings, and would need to know about the short and
long ParkableStrings). So the second one is selected here.

The new overhead is:
- Short ParkableStrings:
  - sizeof(ParkableStringImpl) == 3 * sizeof(void*) compared to
    WTF::String
  - savings of sizeof(ParkableStringImpl::Metadata) compared to
    ParkableString
- Long ParkableStrings: 0 vs the previous overhead (by replacing a
  std::unique_ptr<SecureDigest> with a
  std::unique_ptr<ParkableStringImpl::ParkableMetadata>)

Given that client code had to keep a ParkableString *and* a String to
handle short ones, the net cost is 2*sizeof(void*) for short strings,
and no code duplication.

As a consequence, ParkableString can now be used for short and long
strings without any custom code. CharacterData is converted
accordingly. A forthcoming CL will evaluate using ParkableString for
*all* CharacterData instances.

Also some cleanup:
- "git cl lint" warnings: include-what-you-use and explicit constructor
- Move mutex runtime assertions to compile-time checks.
- Make the ordering in parkable_string.cc consistent with the .h

Bug: 1059268, 877044
Change-Id: Ic397751970c6beeaf9d524102f50d5d71ec4a0ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2091494
Commit-Queue: Benoit L <lizeb@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748199}