| Summary: | [bmalloc] The tracking of freeableMemory of Heap doesn't count Chunk's metadata size. | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Basuke Suzuki <Basuke.Suzuki> | ||||||
| Component: | bmalloc | Assignee: | Basuke Suzuki <Basuke.Suzuki> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | Basuke.Suzuki, commit-queue, ggaren, msaboff, saam, webkit-bug-importer, ysuzuki | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Basuke Suzuki
2019-11-12 17:11:18 PST
Created attachment 383411 [details]
PATCH
This is first part of the fix of the bug. I couldn't figure out that yet, but at least this must be right fix.
We did an internal research to log out freeable memory and footprint on each scavenge. This issue was found on that process. Comment on attachment 383411 [details]
PATCH
I think freeable memory is tracking the size of memory usable for allocation. So I don't think we need to add/sub Chunk's metadata size. Is my understanding correct?
https://github.com/WebKit/webkit/blob/5f3db1e7192810f53c4668f01ecb4e4fdbb3dc18/Source/bmalloc/bmalloc/Heap.cpp#L141-L157 This is the former implementation of feeableMemory. So you are right. Then here is the issue: https://github.com/WebKit/webkit/blob/master/Source/bmalloc/bmalloc/Heap.cpp#L286 When allocating a chunk in allocateSmallChunk(), chunkSize is added to freeableMemory. It should add only sizes of free pages. (In reply to Basuke Suzuki from comment #4) > https://github.com/WebKit/webkit/blob/ > 5f3db1e7192810f53c4668f01ecb4e4fdbb3dc18/Source/bmalloc/bmalloc/Heap. > cpp#L141-L157 > > This is the former implementation of feeableMemory. So you are right. Then > here is the issue: > > https://github.com/WebKit/webkit/blob/master/Source/bmalloc/bmalloc/Heap. > cpp#L286 > > When allocating a chunk in allocateSmallChunk(), chunkSize is added to > freeableMemory. It should add only sizes of free pages. Right. This looks wrong, nice. Created attachment 383677 [details]
PATCH
Comment on attachment 383677 [details]
PATCH
r=me
Comment on attachment 383677 [details] PATCH Clearing flags on attachment: 383677 Committed r252519: <https://trac.webkit.org/changeset/252519> All reviewed patches have been landed. Closing bug. |