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

Reduce how long it takes to walk the varrefs in an expression #8770

Merged
merged 1 commit into from
Aug 31, 2017

Conversation

jsternberg
Copy link
Contributor

@jsternberg jsternberg commented Aug 30, 2017

This is used quite a bit to determine which fields are needed in a
condition. When the condition gets large, the memory usage begins to
slow it down considerably and it doesn't take care of duplicates.

  • Rebased/mergable
  • Tests pass
  • CHANGELOG.md updated

@jsternberg
Copy link
Contributor Author

Benchmark comparison:

benchmark                old ns/op     new ns/op     delta
BenchmarkExprNames-8     131087        8389          -93.60%

benchmark                old allocs     new allocs     delta
BenchmarkExprNames-8     308            10             -96.75%

benchmark                old bytes     new bytes     delta
BenchmarkExprNames-8     135424        240           -99.82%

@jsternberg jsternberg requested a review from e-dard August 30, 2017 15:38
Copy link
Contributor

@e-dard e-dard left a comment

Choose a reason for hiding this comment

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

LGTM 👍

In my testing, this improved a SELECT ... WHERE (lots of ORs) ORDER BY TIME DESC LIMIT 1 query by around 57%.

@jsternberg jsternberg force-pushed the js-reduce-walk-refs-memory-usage branch from 8c11eb6 to 8d384c1 Compare August 31, 2017 14:08
This is used quite a bit to determine which fields are needed in a
condition. When the condition gets large, the memory usage begins to
slow it down considerably and it doesn't take care of duplicates.
@jsternberg jsternberg force-pushed the js-reduce-walk-refs-memory-usage branch from 8d384c1 to 466fc90 Compare August 31, 2017 14:33
@jsternberg jsternberg merged commit 0ef033f into master Aug 31, 2017
@jsternberg jsternberg deleted the js-reduce-walk-refs-memory-usage branch August 31, 2017 15:15
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