Bug 235484 - LocalStorage values should be 8-bit strings in memory if possible
Summary: LocalStorage values should be 8-bit strings in memory if possible
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ben Nham
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-01-22 15:29 PST by Ben Nham
Modified: 2022-02-03 14:13 PST (History)
3 users (show)

See Also:


Attachments
Patch (1.70 KB, patch)
2022-01-22 15:34 PST, Ben Nham
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ben Nham 2022-01-22 15:29:37 PST
LocalStorage values should be 8-bit strings in memory if possible
Comment 1 Ben Nham 2022-01-22 15:34:19 PST
Created attachment 449739 [details]
Patch
Comment 2 Chris Dumez 2022-01-24 07:26:12 PST
Comment on attachment 449739 [details]
Patch

r=me
Comment 3 Radar WebKit Bug Importer 2022-01-24 11:13:08 PST
<rdar://problem/87980407>
Comment 4 Ben Nham 2022-02-03 13:34:20 PST
Comment on attachment 449739 [details]
Patch

This ended up being only neutral on Membuster. One of the websites in the benchmark has 1.4MB of LocalStorage values in its database. Unfortunately, the values are almost-ASCII but not quite; the site seems to store WebFont css in LocalStorage, and the CSS has a non-ASCII smiley face in it:

@font-face {
  // ...
  src: local('☺︎'),
  // ...
}

See https://www.paulirish.com/2009/bulletproof-font-face-implementation-syntax/#smiley for more details. So the value can't actually be parsed into an 8-bit string and this optimization does nothing for that website.

This is still worth doing though, so I'll land it.
Comment 5 Yusuke Suzuki 2022-02-03 14:02:17 PST
For this kind of long-living values, making it 8bit as much as possible really makes sense! Nice
Comment 6 EWS 2022-02-03 14:13:52 PST
Committed r289078 (246784@main): <https://commits.webkit.org/246784@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 449739 [details].