-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
go/vt/sqlparser: improve performance in TrackedBuffer formatting #16364
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ package sqlparser | |
|
||
import ( | ||
"fmt" | ||
"strconv" | ||
"strings" | ||
) | ||
|
||
|
@@ -211,7 +212,32 @@ func (buf *TrackedBuffer) astPrintf(currentNode SQLNode, format string, values . | |
} | ||
} | ||
case 'd': | ||
buf.WriteString(fmt.Sprintf("%d", values[fieldnum])) | ||
switch v := values[fieldnum].(type) { | ||
case int: | ||
buf.WriteInt(int64(v)) | ||
case int8: | ||
buf.WriteInt(int64(v)) | ||
case int16: | ||
buf.WriteInt(int64(v)) | ||
case int32: | ||
buf.WriteInt(int64(v)) | ||
case int64: | ||
buf.WriteInt(v) | ||
case uint: | ||
buf.WriteUint(uint64(v)) | ||
case uint8: | ||
buf.WriteUint(uint64(v)) | ||
case uint16: | ||
buf.WriteUint(uint64(v)) | ||
case uint32: | ||
buf.WriteUint(uint64(v)) | ||
case uint64: | ||
buf.WriteUint(v) | ||
case uintptr: | ||
buf.WriteUint(uint64(v)) | ||
default: | ||
panic(fmt.Sprintf("unexepcted TrackedBuffer type %T", v)) | ||
} | ||
case 'a': | ||
buf.WriteArg("", values[fieldnum].(string)) | ||
default: | ||
|
@@ -288,14 +314,26 @@ func areBothISExpr(op Expr, val Expr) bool { | |
// WriteArg writes a value argument into the buffer along with | ||
// tracking information for future substitutions. | ||
func (buf *TrackedBuffer) WriteArg(prefix, arg string) { | ||
length := len(prefix) + len(arg) | ||
buf.bindLocations = append(buf.bindLocations, BindLocation{ | ||
Offset: buf.Len(), | ||
Length: len(prefix) + len(arg), | ||
Length: length, | ||
}) | ||
buf.Grow(length) | ||
buf.WriteString(prefix) | ||
buf.WriteString(arg) | ||
} | ||
|
||
// WriteInt writes a signed integer into the buffer. | ||
func (buf *TrackedBuffer) WriteInt(v int64) { | ||
buf.WriteString(strconv.FormatInt(v, 10)) | ||
} | ||
|
||
// WriteUint writes an unsigned integer into the buffer. | ||
func (buf *TrackedBuffer) WriteUint(v uint64) { | ||
buf.WriteString(strconv.FormatUint(v, 10)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that this is still allocating memory when the small-number optimization doesn't trigger, so that isn't great. I think it may be time to leave behind the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I assume you're thinking something more specialized like what |
||
} | ||
|
||
// ParsedQuery returns a ParsedQuery that contains bind | ||
// locations for easy substitution. | ||
func (buf *TrackedBuffer) ParsedQuery() *ParsedQuery { | ||
|
@@ -335,7 +373,6 @@ func UnescapedString(node SQLNode) string { | |
buf.SetEscapeNoIdentifier() | ||
node.Format(buf) | ||
return buf.String() | ||
|
||
} | ||
|
||
// CanonicalString returns a canonical string representation of an SQLNode where all identifiers | ||
|
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.
As a potential follow up:
I got here because
WriteArg
stood out a bit in a pprof I was looking at for heap allocations, and the other thing that can be done in here with some more work is thebindLocations
slice that keeps getting appended to 1 at a time.So we might want to do a thing where we count up the args first so can we grow the slice, then append to the slice. So adding a method like,
GrowArgs(size)