Summary: | Fix comment in NewSpace.h | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Zoltan Herczeg <zherczeg> | ||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED INVALID | ||||||
Severity: | Normal | CC: | barraclough, ggaren, oliver | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | 65458 | ||||||
Bug Blocks: | |||||||
Attachments: |
|
Description
Zoltan Herczeg
2011-08-07 23:17:21 PDT
Created attachment 103203 [details]
patch
cc'ing GC experts Comment on attachment 103203 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=103203&action=review > Source/JavaScriptCore/heap/NewSpace.h:88 > + // [ 128 - preciseStep, 256... 1024 ) I don't think this is true: the first imprecise size class is exactly 128, not 128 - preciseStep. > I don't think this is true: the first imprecise size class is exactly 128, not 128 - preciseStep. See bug 65458 (Depends on). The bug was that an allocation of 103 bytes is bigger than 95, which is the end of last precise class on 64 bit. So it is allocated from the first inprecise class. Or should it go to a precise class? > See bug 65458 (Depends on). The bug was that an allocation of 103 bytes is bigger than 95, which is the end of last precise class on 64 bit. So it is allocated from the first inprecise class. Or should it go to a precise class?
Where to allocate is one question. Independently, this comment describes the size of the cells in the class. That size is 128, not 128 - preciseStep.
> Where to allocate is one question. Independently, this comment describes the size of the cells in the class. That size is 128, not 128 - preciseStep.
Are you sure? The comment said:
// [ 128, 256... 1024 )
The size of the first block is really 128, but the second is 128 as well, not 256, and the last is 128 as well, not 1024. Instead I think the comment describes the ranges, the first range is [128-256), the second is [256-384) ...
The growth rate is clearly linear:
static const size_t preciseCutoff = 128;
static const size_t impreciseStep = preciseCutoff;
return m_impreciseSizeClasses[(bytes - 1) / impreciseStep];
(gdb) disp cellSize (gdb) run [Switching to process 3491] Running… 3: cellSize = 32 (gdb) c Continuing. 3: cellSize = 64 (gdb) Continuing. 3: cellSize = 96 (gdb) Continuing. 4: cellSize = 128 (gdb) Continuing. 4: cellSize = 256 (gdb) Continuing. 4: cellSize = 384 (gdb) Continuing. 4: cellSize = 512 (gdb) Continuing. 4: cellSize = 640 (gdb) Continuing. 4: cellSize = 768 (gdb) Continuing. 4: cellSize = 896 Oh, those brackets were confused me. [a,b) in mathematics means a range of numbers, where x >= a and x < b. Sorry for the misunderstanding. |