Skip to content
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

add vec as PersistentVector.fromArray #198

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alkhe
Copy link

@alkhe alkhe commented Mar 7, 2017

I was running some performance tests on mori vs. immutable, and found that we could make vector creation from a JS array at least two times faster:

https://gist.github.com/edge/571bc3da31cb37d98addd78cc937f4d6

113 ms M linear conj
56 ms M atomic instance (apply)
28 ms M atomic instance (vec)
882 ms M js->clj
522 ms I linear push
238 ms I batched linear push
83 ms I atomic instance
215 ms I fromJS

The idiomatic way to create a vector with 100 zeroes (yielding 113 ms):

let v = vector()
for (let i = 0; i < 100; i++) {
	v = conj(v, 0)
}

A slightly more efficient way (yielding 56 ms):

const v = vector.apply(null, Array(100).fill(0))

However, this approach is limited to the number of arguments per call that the Javascript engine supports (though still quite a high number).

The arguments object is actually quite slow in Javascript, and this is well documented.

By skipping certain checks when we know that we are instantiating from an array, we can eschew the expensive conversion ( [& args]), avoid the function call limitation, and provide a cleaner interface (yielding 28 ms):

const v = vec(Array(100).fill(0))

I also experimented with using cljs.core/vec, but found it even slower than the apply strategy, perhaps because PersistentVector.fromArray is that much more efficient.

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.

1 participant