Uh oh!
There was an error while loading. Please reload this page.
String mapi ~2.5x perf improvement (v2) - #9482
Conversation
…nction is in scope
Uh oh!
There was an error while loading. Please reload this page.
Hmm, CI errors:
That's this test: [<Test>]memberthis.MapI()=lete1= String.mapi (fun i c -> char(int c + i))"foo"
Assert.AreEqual("fpq", e1)lete2= String.mapi (fun i c -> c)null Assert.AreEqual("", e2)And yes, these were wrong, incorrectly copied & changed the code from the BDN project. I've added more tests since we're now unrolling by |
Another sourcelink fail?
Not sure what causes this, doesn't seem related. |
abelbraaksma
commented
Jun 18, 2020
All's green now, newly added tests passed. |
Uh oh!
There was an error while loading. Please reload this page.
abelbraaksma
commented
Jun 24, 2020
@KevinRansom, the unrolling was removed, as discussed, all requested changes implemented and green. Good to go? |
abelbraaksma
commented
Jul 3, 2020
@cartermp, you merged master into this, I think this is now ready? |
cartermp
commented
Jul 5, 2020
This will require a re-review by @KevinRansom to merge |
abelbraaksma
commented
Jul 11, 2020
/cc @KevinRansom |
abelbraaksma
commented
Jul 25, 2020
Thanks @KevinRansom! |
KevinRansom
commented
Jul 25, 2020
@abelbraaksma , happy to be of service. |
* Move `String.length` to the top of its module so that the `length` function is in scope * Using known length for mapping, and unrolling x2 to improve perf of String.mapi by 2.5x * Fix two unapparent errors in the unroll loop, fix nit on parens * Add String.mapi tests for all new unrolling corner cases, and some general ones * Add side-effect test, this would fail if sequentiality of String.mapi is broken * Remove "foo" from tests * Remove unrolling of loop in String.mapi Co-authored-by: Phillip Carter <pcarter@fastmail.com>
See for details of timings that motivated this improvement: #9390 (comment).
TLDR: using a
char []improves ~2.1x,then 2x unrolling brings this to ~2.5x(edit: no unrolling will be done, the JIT should do that). In other words, if it took 1 second before, it now takes 0.4 seconds.Dependent on: #9481.