Bug 207673

Summary:

[JSC] Compact JITCodeMap by storing BytecodeIndex and CodeLocation separately

Product:

WebKit

Reporter:

Yusuke Suzuki <ysuzuki>

Component:

New Bugs

Assignee:

Yusuke Suzuki <ysuzuki>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

benjamin, cdumez, cmarcelo, dbates, ews-watchlist, keith_miller, mark.lam, msaboff, rmorisset, saam, tzagallo, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

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.