string-validate-input hits these 24k times in total.
Created attachment 715343 [details] [diff] [review] Patch Patch inlines comparison of linear strings which are <64 code units in length. The vast majority of comparisons should fall into this class and the inline path should fall under the vmcall overhead for those cases. To avoid register pressure on x86, the comparison loop stores the lengths of both strings in one register, and uses some bit tricks (described in comments) to do the comparison.
Comment on attachment 715343 [details] [diff] [review] Patch Review of attachment 715343 [details] [diff] [review]: ----------------------------------------------------------------- I'm not sure it's worth it. I see a <= 0.2 ms win on validate-input on x86 with --no-jm with/without parallel compilation, no other benchmarks we know need this, and the other JITs also don't specialize this case. What do you think?
I think having a stub is worth it, if only to convey the (String, String) specialization for the compare. Once we get rid of stack typesets this info will have to come from here. Maybe the fancy assembly in the stub may not be ;) OTOH, I do think the fancy assembly would be interesting to use in Ion.
Ion should be able to determine the two operands are strings, even without StackTypeSets. There shouldn't be any loss in functionality with bug 804676, it will just be restructuring things so that the types we now explicitly track and keep around in StackTypeSets are instead encoded in the MIR.
In that case we can probably leave this be for now. I'm not convinced we shouldn't have a stub, though. If for whatever reason we have string-heavy code that doesn't spend a lot of time in Ion, we'll end up taking a _really_ slow path for it every time - with stack iter, a table lookup, stub-attachment checks, the whole lot. We should try to avoid that just on principle.
Comment on attachment 715343 [details] [diff] [review] Patch Forgot to cancel this review a long time ago..
You need to log in before you can comment on or make changes to this bug.