-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
case/when eqq calls do not cache #3513
Comments
Seems like making a SearchConstantSite like InvokeSite would solve this wouldn't it? For some reason I thought there was already some caching of constants. Or was that not for SearchConstantInstr? |
EqqInstr is only used by case/when right now, I believe. It's basically the At some point we'll want to explore optimizing case/when itself for homogeneous types, like all symbols or all fixnums. I think that will have to happen in the builder, creating a special jump table instruction that can do O(1) jumps. |
oh the caching of === itself. Sorry I misread that. This might be over simplistic but why is this even its own instruction? Seems like we could have just made this a callInstr for ===. There is something with undefined I don't get (so maybe it is more special than a call) but this would eliminate an instr and get caching for free. It strikes me simple specialization of calls for very specific calls should just be emitted differently in the JIT (unless it is much faster in interp too in which case the extra complexity of having an instr is worth it). As you say we also want to optimize monotyped case/whens but that will need to happen earlier (in Builder or a pass). |
ok I looked at this briefly. eqqinstr has some extra semantics beyond === but they are pretty simple to work around in IRBuilder. If rhs of === is undefined it represents a valueless case/when which is for truthiness. In this case I think we can build something explicit with btrue and then for ordinary case we emit the case with a callinstr of ===. The secondary concern with this change is ability to detect case if we want to optimize cases in a compiler pass. I would argue ther is not a huge difference in an if/else pattern which has eqq and one which has callinstr of ===. The callinstr version will be a slightly more complicated check but it will also be slightly more generic. So I think removing eqqinstr in favor of a callinstr will:
|
I pushed a change to at least include a call site in the eqq logic. before:
after:
This helps basically all cases where the cache is hit every time. We're still rather far off from the optimized cases in 1.7 though (the O(1) hacks I made):
I have no idea how frequently these simple cases (fixnums, one-char str, one-char sym) come up in the real world, though. |
This is fine but I am now liking the idea of killing eqqinstr but I guess that will just end up being lots of code deletions... |
I'm going to mark this fixed and open a separate bug for ongoing optimizations. |
Perhaps not a major loss but eqq calls in case/when do not do any caching currently, in either the interpreter or the JIT.
EqqInstr and its equivalent code in the JIT do not do any caching, and just call IRRuntimeHelpers.isEQQ.
The text was updated successfully, but these errors were encountered: