Skip to content

Add Watch update perf test #1059

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

Merged
merged 5 commits into from
Jul 30, 2018
Merged

Conversation

schmidt-sebastian
Copy link
Contributor

@schmidt-sebastian schmidt-sebastian commented Jul 28, 2018

When we have n-updates for n targets, we update the DbTargetGlobal store n times. In the multi-tab branch, this even adds n reads. This perf tests will be used in my follow-up PR that gets rid of these extra updates.

This test is so slow at 100 queries per round that I reduced it to 25.

Copy link
Contributor

@mikelehen mikelehen left a comment

Choose a reason for hiding this comment

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

LGTM with nits.

let steps = spec().withGCEnabled(false);

for (let i = 1; i <= STEP_COUNT; ++i) {
const collPath = `collection/${i}/coll`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you use a different subcollection for each step_count ? Please comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added comment. I want the loop iterations to be distinct.


for (let i = 1; i <= STEP_COUNT; ++i) {
const collPath = `collection/${i}/coll`;
const matchingDoc = doc(`${collPath}/${i}`, ++currentVersion, {
Copy link
Contributor

Choose a reason for hiding this comment

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

given you're already using a separate subcollection, does the doc name need to be unique too or could this just be ${collPath}/match or something ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that works. Done.


const queries = [];

for (let j = 0; j < queriesPerStep; ++j) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Comments explaining each section of the test would help me read it faster and be sure I was understanding the intention... E.g.

// Create queriesPerStep listens, each against collPath but with a unique query constraint.

@schmidt-sebastian schmidt-sebastian merged commit 5c485dc into master Jul 30, 2018
@schmidt-sebastian schmidt-sebastian deleted the mrschmidt-moreperftests branch August 3, 2018 17:15
@firebase firebase locked and limited conversation to collaborators Oct 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants