-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Byte code size and serialized byte code size optimizations (15-20% serialized byte code size reduction) #3594
Conversation
c45931a
to
31cc8d6
Compare
@@ -8889,7 +8889,7 @@ const byte * InterpreterStackFrame::OP_ProfiledLoopBodyStart(const byte * ip) | |||
return JavascriptOperators::OP_LdModuleSlot(playout->SlotIndex1, playout->SlotIndex2, scriptContext); | |||
} | |||
|
|||
inline void InterpreterStackFrame::OP_StModuleSlot(Var instance, int32 slotIndex1, int32 slotIndex2, Var value) | |||
inline void InterpreterStackFrame::OP_StModuleSlot(Var instance, uint32 slotIndex1, uint32 slotIndex2, Var value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Var instance [](start = 55, length = 12)
instance
param unused?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah.. it is borrowing the SET_ELEM_ENVSLOTNonVar in the interpreter dispatch, which to clean up will require more change. I will do that separately.
namespace JsUtil | ||
{ | ||
template <typename TAllocator> | ||
class LineOffsetCache |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LineOffsetCache [](start = 10, length = 15)
Any changes to this code besides splitting into .h and .cpp under lib/runtime/base and removing the JsUtil namespace?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Splitting the data from charOffset/byteOffset pair to separate list, so that the byteOffset don't need to be stored if it is ASCII-7 (which charOffset == byteOffset)
@@ -64,7 +64,7 @@ namespace Js | |||
const int magicStartOfPropIdsOfFormals = *(int*)"pif["; | |||
const int magicEndOfPropIdsOfFormals = *(int*)"]pif"; | |||
const int magicStartOfSlotIdToNestedIndexArray = *(int*)"sni["; | |||
const int magicEndOfSlotIdToNestedIndexArray = *(int*)"]sni" | |||
const int magicEndOfSlotIdToNestedIndexArray = *(int*)"]sni"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
; [](start = 64, length = 1)
wait... how was this not an error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code is only enabled explicitly to debug issues. No official build has this code enabled.
bool has_slotIdInCachedScopeToNestedIndexArray : 1; | ||
bool has_debuggerScopeSlotArray : 1; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: blank line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
ctStringTemplateCallsite = 10, | ||
ctPropertyString16 = 11, | ||
ctInt16 = 12, | ||
ctInt8 = 13, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not group the three int size constant types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess since the byte code is going to be updated already, I might as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sigh... I will have to renumber them all
@@ -1784,45 +1813,35 @@ class ByteCodeBufferBuilder | |||
return sizeof(serialization_alignment TStructType); | |||
} | |||
|
|||
uint32 AddPropertyIdOfFormals(BufferBuilderList & builder, FunctionBody * function) | |||
uint32 AddPropertyIdOfFormals(BufferBuilderList & builder, PropertyIdArray * propIds, FunctionBody * function) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
propIds [](start = 81, length = 7)
Assert(propIds != nullptr)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't feel like the assert serves much value.....
@@ -2365,6 +2413,11 @@ class ByteCodeBufferReader | |||
return ReadConstantSizedInt32(buffer, remainingBytes, value); | |||
} | |||
|
|||
const byte * ReadConstantSizedUInt32(const byte * buffer, uint * value) | |||
{ | |||
return ReadConstantSizedInt32(buffer, (int *)value); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Int32 [](start = 32, length = 5)
mismatch of ReadUInt32 (name) and ReadInt32 (returned)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The uint return is in out param.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…rialized byte code size reduction) Byte code size reductions: - Variable slot index size in layout Serialized byte code size reductions: - Move has loopHeaderArray and has asmJsInfo into the bit field (instead of a byte) - Don't emit field that has non-zero default value. - Use the defined fields to indicate existing of more field to reduce serialized byte code size - Use constant type to indicate whether it is property string or not instead of an extra boolean - Add int8 and int16 constant type for small int encoding in the constant table - Separate out the byte offset from the character offset in the LineOffsetCache so that we can omit them if the source is purely ASCII-7 -Reduce by half the memory used by the LineOffsetCache for ASCII-7 sources, and it reduces the serialized byte code size as well. - Remove unused sourceSpans from byte code serialization. - Serialize the bit flag with constant encoding (since the bit flag always as a large value) Also: - Clean up unused Reg2WithICIndex and REg1Int2 layout - Move LineOffsetCache to Runtime/Base instead of Common/DataStructures, and de-templatize it as we only use Recycler as the allocator anyway.
… optimizations (15-20% serialized byte code size reduction) Merge pull request #3594 from curtisman:bytecode Byte code size reductions: - Variable slot index size in layout Serialized byte code size reductions: - Move has loopHeaderArray and has asmJsInfo into the bit field (instead of a byte) - Don't emit field that has non-zero default value. - Use the defined fields to indicate existing of more field to reduce serialized byte code size - Use constant type to indicate whether it is property string or not instead of an extra boolean - Add int8 and int16 constant type for small int encoding in the constant table - Separate out the byte offset from the character offset in the LineOffsetCache so that we can omit them if the source is purely ASCII-7 -Reduce by half the memory used by the LineOffsetCache for ASCII-7 sources, and it reduces the serialized byte code size as well. - Remove unused sourceSpans from byte code serialization. - Serialize the bit flag with constant encoding (since the bit flag always as a large value) Also: - Clean up unused Reg2WithICIndex and REg1Int2 layout - Move LineOffsetCache to Runtime/Base instead of Common/DataStructures, and de-templatize it as we only use Recycler as the allocator anyway.
Byte code size reductions:
Serialized byte code size reductions:
-Reduce by half the memory used by the LineOffsetCache for ASCII-7 sources, and it reduces the serialized byte code size as well.
Also: