-
-
Notifications
You must be signed in to change notification settings - Fork 213
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
[stats] costed index scan perf #2421
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
sql/stats/filter.go
Outdated
func Union(s1, s2 sql.Statistic) sql.Statistic { | ||
return s1 | ||
func Union(b1, b2 []sql.HistogramBucket, types []sql.Type) ([]sql.HistogramBucket, error) { | ||
var ret []sql.HistogramBucket |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might want to allocate the full potential capacity here to avoid a bunch of mallocs during the loop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kk
func Intersect(s1, s2 sql.Statistic) sql.Statistic { | ||
return s1 | ||
func Intersect(b1, b2 []sql.HistogramBucket, types []sql.Type) ([]sql.HistogramBucket, error) { | ||
var ret []sql.HistogramBucket |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment about allocations. Could also consider a pool.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it starts to show up on profiles again i can add a pool, so far sysbench and tpc-c don't seem to execute this a lot
Histogram copying is expensive. Instead pass and mutate references. We have to use a different struct type to load stats from JSON in order to support histogram interface generalization.
related Dolt-side: dolthub/dolt#7666