Bug 194469 – [WTF] Use BufferInternal StringImpl if substring StringImpl takes more memory

• Component: New Bugs
• Status: RESOLVED FIXED

[Yusuke Suzuki]

[WTF] Use BufferInternal StringImpl if substring StringImpl takes more memory

[Yusuke Suzuki]

Created attachment 361596 [details]
Patch

[Geoffrey Garen]

Comment on attachment 361596 [details]
Patch

r=me

[Yusuke Suzuki]

It shows ~1% improvement in RAMification on my MBP.
(6 runs, 35159804.0 v.s. 34825966.166666664).

[EWS Watchlist]

Comment on attachment 361596 [details]
Patch

Attachment 361596 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/11088571

New failing tests:
imported/w3c/web-platform-tests/mediacapture-record/MediaRecorder-constructor.html

[EWS Watchlist]

Created attachment 361601 [details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[Yusuke Suzuki]

(In reply to Build Bot from comment #5)
> Created attachment 361601 [details]
> Archive of layout-test-results from ews106 for mac-highsierra-wk2
> 
> The attached test failures were seen while running run-webkit-tests on the
> mac-wk2-ews.
> Bot: ews106  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

It crashes with the softlinking initialization. I think it is unlikely that this patch is related to softlinking.

[Yusuke Suzuki]

Committed r241237: <https://trac.webkit.org/changeset/241237>

[Radar WebKit Bug Importer]

<rdar://problem/47939638>

[WebKit Commit Bot]

Re-opened since this is blocked by bug 194474

[Yusuke Suzuki]

(In reply to WebKit Commit Bot from comment #9)
> Re-opened since this is blocked by bug 194474

It shows great improvement in some tests, but it also shows significant memory increase in some tests (like, WSL, date-format-xparb etc.). Overall, RAMification gets 1% win, but increasing significant memory footprint in some tests is not good. If we can get the benefit without increasing memory usage in these tests, it is very nice.
I rollout this patch now, and I'm now investigating...

[Yusuke Suzuki]

(In reply to Yusuke Suzuki from comment #3)
> It shows ~1% improvement in RAMification on my MBP.
> (6 runs, 35159804.0 v.s. 34825966.166666664).

Not correct. I accidentally compared to n-hours older version, and during this, we have a patch with significant change. And it seems that this patch is not directly related to this significant memory bloat. Investigating.

[Yusuke Suzuki]

After the measurement, it is found that this patch is unrelated to the recent regression in date-format-xparb and WSL. rolling in again.

[Yusuke Suzuki]

Committed r241242: <https://trac.webkit.org/changeset/241242>

[Geoffrey Garen]

(In reply to Yusuke Suzuki from comment #12)
> After the measurement, it is found that this patch is unrelated to the
> recent regression in date-format-xparb and WSL. rolling in again.

Do we have a bug report tracking that (unrelated) regression?

[Yusuke Suzuki]

(In reply to Geoffrey Garen from comment #14)
> (In reply to Yusuke Suzuki from comment #12)
> > After the measurement, it is found that this patch is unrelated to the
> > recent regression in date-format-xparb and WSL. rolling in again.
> 
> Do we have a bug report tracking that (unrelated) regression?

No, we don't have that yet. It is good idea! I'll open the bug for that :D

[Yusuke Suzuki]

(In reply to Yusuke Suzuki from comment #15)
> (In reply to Geoffrey Garen from comment #14)
> > (In reply to Yusuke Suzuki from comment #12)
> > > After the measurement, it is found that this patch is unrelated to the
> > > recent regression in date-format-xparb and WSL. rolling in again.
> > 
> > Do we have a bug report tracking that (unrelated) regression?
> 
> No, we don't have that yet. It is good idea! I'll open the bug for that :D

Opened. https://bugs.webkit.org/show_bug.cgi?id=194476