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 support for setting multiple values at once in ViewHistory #4692

Merged
merged 2 commits into from
Apr 28, 2014
Merged

Add support for setting multiple values at once in ViewHistory #4692

merged 2 commits into from
Apr 28, 2014

Conversation

Snuffleupagus
Copy link
Collaborator

Currently for every call to updateViewarea, ViewHistory.set is called five times, which means that the database is converted to a string and written to storage five times in succession.

This PR adds a method, setMultiple, that sets all values before the database is written to storage.

Generally the value of micro-benchmarks can be questioned, but to get at least some confirmation that this PR is meaningful I've tried comparing the current code with this patch: http://jsperf.com/viewhistory.
Based on the consistency of the results across various browsers, it seems probable that this PR does reduce the time taken to update the history.

@yurydelendik
Copy link
Contributor

/botio-windows preview

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_preview from @yurydelendik received. Current queue size: 0

Live output at: http://107.22.172.223:8877/3c09ed42b8c568e/output.txt

yurydelendik added a commit that referenced this pull request Apr 28, 2014
Add support for setting multiple values at once in ViewHistory
@yurydelendik yurydelendik merged commit 42ed902 into mozilla:master Apr 28, 2014
@yurydelendik
Copy link
Contributor

Thank you

@Snuffleupagus Snuffleupagus deleted the viewHistory-setMultiple branch April 28, 2014 07:39
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