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

Use fixed-length array in idb/indexeddb allDocs #7040

Merged
merged 1 commit into from Jan 25, 2018

Conversation

alxndrsn
Copy link
Member

@alxndrsn alxndrsn commented Jan 25, 2018

I've been benchmarking db.allDocs({ keys: someIds }), and this PR may have a significant effect for some users.

Initialising arrays with a known length seems to be much faster in Chrome than using []. In Firefox, there seems to be little difference.

https://jsperf.com/random-array-inserts

Running the same benchmark directly in the Chrome console* seems to have even more marked differences, but presumably there's some other factor at play:

screen shot 2018-01-25 at 14 27 55

N.B. COUNT = 20000

*application which includes PouchDB was also loaded in this browser window.

@daleharvey
Copy link
Member

So there has been efforts to try and get our performance tests ready for production, fail CI if we get performance regressions and a dashboard of what our performance looks like. If you are interested in doing that would be happy to discuss in a new issue.

As for this improvment its a simple change that doesnt complicate the code so happy to merge, thank you :)

Copy link
Member

@daleharvey daleharvey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice change

@daleharvey daleharvey merged commit e47400c into pouchdb:master Jan 25, 2018
@alxndrsn alxndrsn deleted the fixed-length-arrays branch January 26, 2018 05:36
@alxndrsn
Copy link
Member Author

alxndrsn commented Jan 26, 2018

Thanks for merging!

Ref perf testing, I'm very interested in learning more. I'll take a look at the current suite and see if my change had any effect!

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.

None yet

2 participants