Bug 207673 - [JSC] Compact JITCodeMap by storing BytecodeIndex and CodeLocation separately
Summary: [JSC] Compact JITCodeMap by storing BytecodeIndex and CodeLocation separately
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: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-02-12 17:55 PST by Yusuke Suzuki
Modified: 2020-02-12 18:38 PST (History)
12 users (show)

See Also:

Attachments
Patch (7.66 KB, patch)
2020-02-12 18:01 PST, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (7.85 KB, patch)
2020-02-12 18:05 PST, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (7.85 KB, patch)
2020-02-12 18:08 PST, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2020-02-12 17:55:31 PST 
[JSC] Compact JITCodeMap by stroing BytecodeIndex and CodeLocation separately
Comment 1 Yusuke Suzuki 2020-02-12 18:01:56 PST 
Created attachment 390603 [details]
Patch
Comment 2 Mark Lam 2020-02-12 18:05:34 PST 
Comment on attachment 390603 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=390603&action=review

> Source/JavaScriptCore/ChangeLog:3
> +        [JSC] Compact JITCodeMap by stroing BytecodeIndex and CodeLocation separately

/stroing/strong/
Comment 3 Yusuke Suzuki 2020-02-12 18:05:57 PST 
Created attachment 390605 [details]
Patch
Comment 4 Yusuke Suzuki 2020-02-12 18:06:35 PST 
Comment on attachment 390603 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=390603&action=review

>> Source/JavaScriptCore/ChangeLog:3
>> +        [JSC] Compact JITCodeMap by stroing BytecodeIndex and CodeLocation separately
> 
> /stroing/strong/

Thanks, fixing.
Comment 5 Yusuke Suzuki 2020-02-12 18:08:29 PST 
Created attachment 390606 [details]
Patch
Comment 6 Mark Lam 2020-02-12 18:28:17 PST 
Comment on attachment 390606 [details]
Patch

r=me
Comment 7 Yusuke Suzuki 2020-02-12 18:30:12 PST 
Committed r256498: <https://trac.webkit.org/changeset/256498>
Comment 8 Radar WebKit Bug Importer 2020-02-12 18:31:14 PST 
<rdar://problem/59410018>
Comment 9 Robin Morisset 2020-02-12 18:31:45 PST 
Comment on attachment 390606 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=390606&action=review

> Source/JavaScriptCore/jit/JITCodeMap.h:46
> +        m_pointer = MallocPtr<uint8_t>::malloc(sizeof(CodeLocationLabel<JSEntryPtrTag>) * m_size + sizeof(BytecodeIndex) * m_size);

Don't we need to insure some alignment minimum here ? (I would say alignof(CodeLocationLabel<JSEntryPtrTag>)

> Source/JavaScriptCore/jit/JITCodeMap.h:56
> +        return codeLocations()[index - indexes()];

I'm confused about this line. Can you explain why it is not just codeLocations()[index] ?
Comment 10 Yusuke Suzuki 2020-02-12 18:38:35 PST 
Comment on attachment 390606 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=390606&action=review

>> Source/JavaScriptCore/jit/JITCodeMap.h:46
>> +        m_pointer = MallocPtr<uint8_t>::malloc(sizeof(CodeLocationLabel<JSEntryPtrTag>) * m_size + sizeof(BytecodeIndex) * m_size);
> 
> Don't we need to insure some alignment minimum here ? (I would say alignof(CodeLocationLabel<JSEntryPtrTag>)

MallocPtr is just calling malloc. So we can assume usual alignment requirement for malloc here.

>> Source/JavaScriptCore/jit/JITCodeMap.h:56
>> +        return codeLocations()[index - indexes()];
> 
> I'm confused about this line. Can you explain why it is not just codeLocations()[index] ?

index is `BytecodeIndex*` pointing the found entry (or nullptr). `index - indexes()` can calculate the offset.