Skip to content

Faster array equality#456

Merged
paf31 merged 1 commit intomasterfrom
eq-array
Jul 1, 2014
Merged

Faster array equality#456
paf31 merged 1 commit intomasterfrom
eq-array

Conversation

@garyb
Copy link
Copy Markdown
Member

@garyb garyb commented Jul 1, 2014

This isn't ideal, but it's good for a 500ms faster typecheck in ps-in-ps on my machine. The main problem is the PS implementation results in two array.slice(1) calls per each index being checked.

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Jul 1, 2014

The calls to slice are generally an issue - I wonder if there's anything we can do about them. For tail recursive pattern matches, we can probably pass along an index which would be faster.

Anyway, this looks good, thanks.

paf31 added a commit that referenced this pull request Jul 1, 2014
@paf31 paf31 merged commit 9f250a7 into master Jul 1, 2014
@paf31 paf31 deleted the eq-array branch July 1, 2014 15:32
@jdegoes
Copy link
Copy Markdown

jdegoes commented Jul 1, 2014

Indeed, using Array for a list may be an issue without a much smarter optimizer. Much of the existing PS code assumes the cost of unconsing is small, which is not in fact true when the list is backed by an Array.

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Jul 1, 2014

Well we have other options. @taku0 has a repository for 2-3-finger tree based sequences, for example. The problem is that the current cons pattern is baked into the code generator. If we had some sort of active pattern capability, things like that might be easier to use.

Even singly-linked lists implemented as objects might perform better for some use cases. Probably worth looking at.

I'm not sure how we could optimize away the calls to splice in the current optimizer. It would probably have to work in tandem with the code generator.

@garyb
Copy link
Copy Markdown
Member Author

garyb commented Jul 1, 2014

Would we rename the JS Array type and cons/uncons to something else then? Or would we keep that as the default and just change it out for more suitable data structures as needed. It is pretty handy having Arrays as the default sequence from a JS interop point of view.

@garyb
Copy link
Copy Markdown
Member Author

garyb commented Jul 1, 2014

As for optimising the slice, I guess it's another case where we'd need an new IR or expanded AST so we can optimise before JS.

@paf31
Copy link
Copy Markdown
Contributor

paf31 commented Jul 1, 2014

No, I think for interop reasons, Array has to refer to Javascript arrays, but it would be nice to have the option of using a persistent data structure with efficient cons and uncons with nice pattern matching syntax.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants