New issue
Advanced search Search tips

Issue 920194 link

Starred by 1 user
Status: Assigned
Owner:
lizeb@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android
Pri: 3
Type: Bug

Blocking:
issue 877044



Sign in to add a comment

Investigate and recover from malloc() regression in ParkableString

Project Member Reported by lizeb@chromium.org, Jan 9 
From the ParkableString experiment:
https://uma.googleplex.com/variations?sid=4d83ebc6fb3613b488b0bdeecab67e93

Memory.Experimental.Renderer.Malloc.AfterBackgrounded.5min increases by ~500kB on Android. As the ParkableString code only uses malloc() for temporary data in Gzip compression, and reported values include fragmentation in the allocator, we speculate that this may be linked.

As a potential solution, switch to PartitionAlloc for temporary data.
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 11 

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

commit 83ca30af3b48d17cbd19267f6f76f2d6609084fa
Author: Benoît Lizé <lizeb@chromium.org>
Date: Fri Jan 11 09:33:45 2019

blink/bindings: Use PartitionAlloc for compressing strings.

ParkableString compression allocates a temporary buffer inside GzipCompress
using UncheckedMalloc(). From a finch experiment (see bug), there is a
statistically significant increase in malloc() footprint on Android, even though
the only allocations are temporary.
To mitigate the regression, use PartitionAlloc to allocate the temporary data.

This changes:
- compression_utils.cc: take an external output buffer for compression.
- parkable_string.cc: Allocate the temporary buffer with PartitionAlloc, on the
  buffer partition.

Bug: 920194
Change-Id: I57c42f67ea0b09e1ae9137beade4dd0d3c6ef258
Reviewed-on: https://chromium-review.googlesource.com/c/1404083
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
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@{#621954}
[modify] https://crrev.com/83ca30af3b48d17cbd19267f6f76f2d6609084fa/third_party/blink/renderer/platform/bindings/parkable_string.cc
[modify] https://crrev.com/83ca30af3b48d17cbd19267f6f76f2d6609084fa/third_party/blink/renderer/platform/bindings/parkable_string_test.cc
[modify] https://crrev.com/83ca30af3b48d17cbd19267f6f76f2d6609084fa/third_party/zlib/google/compression_utils.cc
[modify] https://crrev.com/83ca30af3b48d17cbd19267f6f76f2d6609084fa/third_party/zlib/google/compression_utils.h

Comment 2 by lizeb@chromium.org, Today (13 hours ago) 

From preliminary experimental data, the regression seems to have disappeared in the latest Chrome Dev release.

The commit above first shipped in Chrome Canary 73.0.3671.2.

From Finch, here is the data for:
- 3667.2: https://uma.googleplex.com/p/chrome/variations/?sid=300baaf828a537fac0161e465021aa65
- 3674.0: https://uma.googleplex.com/p/chrome/variations/?sid=338ac778a726b9115cf6b7e1f5479df1

Keeping in mind that the second one has less data, we still see that the regression seems to have disappeared. Will update the bug later once more data is available.
►

Sign in to add a comment