[JSC] Shrink sizeof(CodeBlock) more
Created attachment 361469 [details] Patch
Attachment 361469 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/ChangeLog:94: Please consider whether the use of security-sensitive phrasing could help someone exploit WebKit: fuzzing [changelog/unwantedsecurityterms] [3] Total errors found: 1 in 37 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 361477 [details] Patch
Attachment 361477 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/ChangeLog:94: Please consider whether the use of security-sensitive phrasing could help someone exploit WebKit: fuzzing [changelog/unwantedsecurityterms] [3] Total errors found: 1 in 37 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 361482 [details] Patch
Attachment 361482 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/ChangeLog:94: Please consider whether the use of security-sensitive phrasing could help someone exploit WebKit: fuzzing [changelog/unwantedsecurityterms] [3] ERROR: Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 2 in 39 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 361482 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=361482&action=review Very nice. r=me with suggestions. > Source/JavaScriptCore/bytecode/CodeBlock.cpp:526 > - for (const auto& instruction : *m_instructions) { > + for (const auto& instruction : instructions()) { Pre-cache instructions() in a local before the loop so that the compiler knows that it won't change on each iteration. > Source/JavaScriptCore/bytecode/CodeBlock.cpp:1074 > + auto instruction = instructions().at(propertyAccessInstructions[i]); Precache instructions in a local before loop. > Source/JavaScriptCore/bytecode/CodeBlock.cpp:1204 > + const auto curInstruction = instructions().at(propertyAccessInstructions[i]); Precache instructions in a local before loop. > Source/JavaScriptCore/bytecode/CodeBlock.cpp:1711 > + auto instruction = instructions().at(bytecodeOffset); Use auto& to make is explicit at a glance that this is a reference. > Source/JavaScriptCore/bytecode/CodeBlock.cpp:1816 > + for (const auto& it : instructions()) { Precache instructions in a local before loop. > Source/JavaScriptCore/bytecode/CodeBlock.cpp:2755 > + for (const auto& instruction : instructions()) { Precache instructions in a local before loop. > Source/JavaScriptCore/bytecode/CodeBlock.cpp:2944 > + for (const auto& instruction : instructions()) { Precache instructions in a local before loop.
Comment on attachment 361482 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=361482&action=review Thank you! >> Source/JavaScriptCore/bytecode/CodeBlock.cpp:526 >> + for (const auto& instruction : instructions()) { > > Pre-cache instructions() in a local before the loop so that the compiler knows that it won't change on each iteration. Fixed. >> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1074 >> + auto instruction = instructions().at(propertyAccessInstructions[i]); > > Precache instructions in a local before loop. Fixed. >> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1204 >> + const auto curInstruction = instructions().at(propertyAccessInstructions[i]); > > Precache instructions in a local before loop. Fixed. >> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1711 >> + auto instruction = instructions().at(bytecodeOffset); > > Use auto& to make is explicit at a glance that this is a reference. Fixed. >> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1816 >> + for (const auto& it : instructions()) { > > Precache instructions in a local before loop. Fixed. >> Source/JavaScriptCore/bytecode/CodeBlock.cpp:2755 >> + for (const auto& instruction : instructions()) { > > Precache instructions in a local before loop. Fixed. >> Source/JavaScriptCore/bytecode/CodeBlock.cpp:2944 >> + for (const auto& instruction : instructions()) { > > Precache instructions in a local before loop. Fixed.
Committed r241222: <https://trac.webkit.org/changeset/241222>
<rdar://problem/47934328>
Comment on attachment 361482 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=361482&action=review >>> Source/JavaScriptCore/bytecode/CodeBlock.cpp:526 >>> + for (const auto& instruction : instructions()) { >> >> Pre-cache instructions() in a local before the loop so that the compiler knows that it won't change on each iteration. > > Fixed. While it’s OK to do this if you prefer it from a stylistic point of view, there is no need to change it for correct behavior. The way a range for loop is defined, the compiler will not and must not call instructions() more than once. An example of an on-line reference that makes this clear is <https://en.cppreference.com/w/cpp/language/range-for>.