YJIT: Remove duplicated information in BranchTarget #7151
Conversation
k0kubun
force-pushed
the
yjit-branch-target
branch
from
19c01c7
to
36765cf
Compare
k0kubun
force-pushed
the
yjit-branch-target
branch
from
36765cf
to
19638b5
Compare
| for target_idx in 0..=1 { | ||
| if let Some(target) = pred_branch.targets[target_idx].as_ref() { | ||
| if target.get_block().as_ref() == Some(blockref) { | ||
| pred_branch.targets[target_idx] = None; |
There was a problem hiding this comment.
So, I remember considering doing this when I first introduced BranchTarget and I got scared that this state where .block=None, but .address=Some... is important. I'd say it's a very very small risk and it's definitely a good time to take such risk right now. In case it does end up being important, could you make a note of this in the commit log? We should just keep this in the back of our mind in case it come up; we'll have to rely on memory for this one.
There was a problem hiding this comment.
Sure, I'll make a note of this in the commit message when squash-merging this.
There was a problem hiding this comment.
summarized my intention in the commit message of 5ce0c13. Let me know if I'm missing something.
In
BranchTarget, you only needid/ctxorblock.blockid/block.ctxat a time. If you encode that constraint in the type, we can save some space by eliminating the duplication.Impact
With macOS stats build on railsbench,
yjit_alloc_sizeis reduced from 9.9MB to 9.0MB.Before
After