• Benoît Lizé's avatar
    blink/bindings: Use PartitionAlloc for zlib's temporary data. · 5e7e1b19
    Benoît Lizé authored
    zlib uses malloc() to allocate temporary data. Unfortunately, on Android,
    jemalloc (malloc()'s underlying implementation) caches temporary data after
    free() in a thread-local cache. As renderers don't allocate from malloc() often
    in background threads, this data is not reclaimed. This increases memory usage
    by up to 256kiB per background compression thread, which is the cause for a
    sizable malloc() memory regression from foreground string compression (see
    linked bug).
    
    The deeper reason for the regression is more complex, see details in the linked
    bug, and
    https://docs.google.com/document/d/1aRIifaHF5l9relq9vHirTOWt2AZoAkDNqHiYZrWGE9A/edit?usp=sharing
    
    It involves some interaction between:
    - jemalloc implementation and memory accounting
    - Android's configuration of jemalloc
    - Chrome's allocation and threading patterns
    - zlib's allocation patterns
    
    Nevertheless, the regression is real, and to mitigate it, use PartitionAlloc to
    allocate zlib's temporary data.
    
    This is not necessary for decompression, as the allocation patterns are not the
    same, and the main thread doesn't have the same issues. As such, only do it for
    compression, to avoid a needlessly complex CL.
    
    On PartitionAlloc vs malloc():
    The only path using partition alloc in this CL in from the renderer process,
    where it is already widely used. The issue is less likely to appear in the
    browser process as malloc() is more widely used there, though further
    investigation may reveal issues.
    
    Android vs everywhere:
    The issue is present at least with jemalloc(), that is Android. However thread
    caches and arena-allocators are used on other platforms as well, and as the
    allocation is not performance-sensitive, using PartitionAlloc everwhere makes
    the code simpler, and protects against potential issues (as we know that this
    specific issue does not arise with PartitionAlloc).
    
    Bug: 931553, 924164
    Change-Id: I9e94e9cea5d51fac67b04fdb1681427e57bfbe1f
    Reviewed-on: https://chromium-review.googlesource.com/c/1472578
    Commit-Queue: Benoit L <lizeb@chromium.org>
    Reviewed-by: default avatarAlexei Svitkine <asvitkine@chromium.org>
    Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#632372}
    5e7e1b19
compression_utils.h 1.99 KB