Use smaller data type to represent the counter fields of FunctionBody. #301
Conversation
lib/Runtime/Base/FunctionBody.h
Outdated
|
|
||
| // Signed integers need keep the sign when promoting | ||
| SignedFieldsStart = 26, | ||
| SerializationIndex = 26, |
There was a problem hiding this comment.
These two are both 26
There was a problem hiding this comment.
SignedFieldsStart is a marker that from that point, it saves signed value. maybe I can remove this, instead, use a byte in the structure to save the position -- there're 3 bytes at the beginning is not used.
There was a problem hiding this comment.
Oh gotcha, I missed that SignedFieldsStart was a label marking the beginning of special fields.
|
With this and your previous change how much does this shrink FunctionBody down on x64? |
|
Ignoring the separately added data for the pointers and counters. I'm curious about sizeof(FunctionBody) for curiosity's sake. |
|
@ianwjhalliday it reduced from 648 to 384 bytes |
leirocks
force-pushed
the
fb_counters_pr
branch
from
bcb3406 to
3a15ec4
Compare
|
@dotnet-bot test this please |
leirocks
force-pushed
the
fb_counters_pr
branch
3 times, most recently
from
92ce4fc to
b19ae63
Compare
|
Nice, that is a significant reduction. |
leirocks
force-pushed
the
fb_counters_pr
branch
2 times, most recently
from
49f445a to
dbffdf4
Compare
leirocks
force-pushed
the
fb_counters_pr
branch
from
dbffdf4 to
106054a
Compare
|
@dotnet-bot test EOL Check please |
| memset(this->m_inlineCaches[i], 0, sizeof(InlineCache)); | ||
| } | ||
| else if(!scriptContext->IsClosed()) | ||
| else if(scriptContext != nullptr && !scriptContext->IsClosed()) |
There was a problem hiding this comment.
In what situation can the scriptContext be nullptr?
There was a problem hiding this comment.
ah, this is not intentional change. previously in order to solve the issue the when FunctionBody::Cleanup sometimes the scriptContext is already gone, I set this to null. So this is not needed any more because when cleaning up we should not be expanding the structure so should not need the recycler from scriptContext anymore.
let me remove the change. thanks.
leirocks
force-pushed
the
fb_counters_pr
branch
from
106054a to
42257a7
Compare
|
@curtisman as discussed, moved following mutable fields out of the structure. and added Assert in case there's any other fields missed. |
|
LGTM |
remove volatile since we made sure there's not sync issue any more
… FunctionBody. Merge pull request #301 from leirocks:fb_counters_pr Use smaller data type to represent the counter fields of FunctionBody. Most of the counters has value less than 256, use single byte to hold them. And can promote to short or full 32 bit integer when necessary.
5 participants
Use smaller data type to represent the counter fields of FunctionBody.
Most of the counters has value less than 256, use single byte to hold them. And can promote to short or full 32 bit integer when necessary.