Skip to content
This repository was archived by the owner on Nov 21, 2023. It is now read-only.

Conversation

mreid-moz
Copy link
Contributor

This has potential performance implications. We should do some testing to see how it performs when we just parse all the json.

Add a test that exposes the problem with lazy json evaluation.
@mreid-moz mreid-moz requested a review from fbertsch June 28, 2017 18:12
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.08%) to 78.888% when pulling 8761b6c on mreid-moz:lazy_bug1376028 into e125d36 on mozilla:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.08%) to 78.888% when pulling 987bb9b on mreid-moz:lazy_bug1376028 into e125d36 on mozilla:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.08%) to 78.888% when pulling fcd8eb8 on mreid-moz:lazy_bug1376028 into e125d36 on mozilla:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.08%) to 78.888% when pulling 79f5197 on mreid-moz:lazy_bug1376028 into e125d36 on mozilla:master.

@mreid-moz
Copy link
Contributor Author

@fbertsch flake8 is slain. ready for a look.

@fbertsch
Copy link
Contributor

@mreid-moz it took 1271s on the old code, and 842s on the new (without lazy parsing). I don't think this is going to have a significant effect, if any at all.

@mreid-moz
Copy link
Contributor Author

What the heck? How can it be faster? Let's verify that we've actually fixed the issue you demonstrated in the notebook in the bug.

@fbertsch fbertsch merged commit b4d68d7 into mozilla:master Jun 29, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants