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

Perf: iterate over docStore.metadata.seq instead of bySeqStore.seq (idb) #3182

Closed
nolanlawson opened this issue Dec 15, 2014 · 4 comments
Closed

Comments

@nolanlawson
Copy link
Member

We'll skip fewer seqs that way. Ought to be faster when we do changes(), e.g. for map/reduce.

All of this recent perf stuff is in the service of cutting down the time of the temp-views test.

@nolanlawson nolanlawson changed the title Perf: iterate over docStore.metadata.seq instead of bySeqStore.seq Perf: iterate over docStore.metadata.seq instead of bySeqStore.seq (idb) Dec 15, 2014
@nolanlawson
Copy link
Member Author

Wow, it's actually a bit slower. Maybe due to the fact that we're iterating a secondary index rather than a primary one??

Firefox: 19397ms -> 21412ms
Chrome: 10802ms -> 12381ms

@nolanlawson
Copy link
Member Author

I ended up writing a migration I'd like to use just for code cleanliness; I'll factor that out into another PR.

@nolanlawson
Copy link
Member Author

Another upside is that we now know we can get rid of the existing 'seq' index, which we had already but weren't using.

nolanlawson added a commit that referenced this issue Dec 15, 2014
I was getting tired of the code complexity with
having to figure out whether `metadata.winningRev`
and `metadata.seq` were there or not. I prefer to
just migrate everything to be sure.

This also removes the docStore's 'seq' index, which
we weren't using and which apparently doesn't offer
any performance benefit.
nolanlawson added a commit that referenced this issue Dec 15, 2014
I was getting tired of the code complexity with
having to figure out whether `metadata.winningRev`
and `metadata.seq` were there or not. I prefer to
just migrate everything to be sure.

This also removes the docStore's 'seq' index, which
we weren't using and which apparently doesn't offer
any performance benefit.
nolanlawson added a commit that referenced this issue Dec 16, 2014
I was getting tired of the code complexity with
having to figure out whether `metadata.winningRev`
and `metadata.seq` were there or not. This adds
a new migration to get everything up to the modern
format.

So technically this migration isn't necessary, but I like
being able to remove a lot of complexity from the runtime
code, at the cost of a moderately-expensive migration
for existing databases.

I also went ahead and refactored the migration logic itself.
It's more readable now.
nolanlawson added a commit that referenced this issue Dec 16, 2014
I was getting tired of the code complexity with
having to figure out whether `metadata.winningRev`
and `metadata.seq` were there or not. This adds
a new migration to get everything up to the modern
format.

So technically this migration isn't necessary, but I like
being able to remove a lot of complexity from the runtime
code, at the cost of a moderately-expensive migration
for existing databases.

I also went ahead and refactored the migration logic itself.
It's more readable now.
@daleharvey
Copy link
Member

We removed the extra index

@nolanlawson nolanlawson mentioned this issue Dec 27, 2014
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

No branches or pull requests

2 participants