[Sqlparser] Pool the yyParserImpls#4175
Conversation
7b58260 to
1a81ed0
Compare
|
EDIT: Fixed. Need to reinitialize Getting some error on ParseNext, but only for one of the tests |
68cf244 to
5589faa
Compare
|
I'm actually surprised you're getting any benefit. For large size allocations like in this case, the cost of zeroing the memory is significant. This is something you're doing in a loop whereas the runtime can do it as a sweep. The variance is probably related to just the cost of obtaining the memory, which will vary depending on fragmentation level. Are you able to get consistent numbers? One experiment you can try (instead of the loop) is to have a static zero-initialized yyParserImpl, and just assign that to the one you got from the pool, which is likely to zero it out more efficiently. |
|
That's a good idea. In any event, there are some issues with holding references to the "backing" stack. I need to sort those out before going forward |
|
And to answer your other question, the numbers are consistent from run to run. |
5589faa to
5c0a216
Compare
go/vt/sqlparser/ast.go
Outdated
There was a problem hiding this comment.
unsurprisingly, this turns out to be a problem:
panic: interface conversion: interface is nil, not sqlparser.SQLNode [recovered]
panic: interface conversion: interface is nil, not sqlparser.SQLNode
goroutine 77 [running]:
testing.tRunner.func1(0xc4206823c0)
/usr/local/go/src/testing/testing.go:742 +0x29d
panic(0x662e60, 0xc42070c180)
/usr/local/go/src/runtime/panic.go:502 +0x229
vitess.io/vitess/go/vt/sqlparser.(*TrackedBuffer).Myprintf(0xc420164900, 0x6b7bd9, 0x9, 0xc42005dd08, 0x1, 0x1)
/home/tahara/vitess/src/vitess.io/vitess/go/vt/sqlparser/tracked_buffer.go:97 +0x3ba
vitess.io/vitess/go/vt/sqlparser.(*Show).Format(0xc4200824b0, 0xc420164900)
/home/tahara/vitess/src/vitess.io/vitess/go/vt/sqlparser/ast.go:1498 +0x1a3
vitess.io/vitess/go/vt/sqlparser.(*TrackedBuffer).Myprintf(0xc420164900, 0x6b60ed, 0x2, 0xc42005de70, 0x1, 0x1)
/home/tahara/vitess/src/vitess.io/vitess/go/vt/sqlparser/tracked_buffer.go:99 +0x3ff
vitess.io/vitess/go/vt/sqlparser.String(0x7fd7c9a1ee68, 0xc4200824b0, 0xc4200824b0, 0x7fd7c9a1ee68)
/home/tahara/vitess/src/vitess.io/vitess/go/vt/sqlparser/ast.go:249 +0xe4
vitess.io/vitess/go/vt/sqlparser.TestValid(0xc4206823c0)
/home/tahara/vitess/src/vitess.io/vitess/go/vt/sqlparser/parse_test.go:1347 +0x20a
testing.tRunner(0xc4206823c0, 0x6d2920)
/usr/local/go/src/testing/testing.go:777 +0xd0
created by testing.(*T).Run
/usr/local/go/src/testing/testing.go:824 +0x2e0
exit status 2
FAIL vitess.io/vitess/go/vt/sqlparser 0.040s
There was a problem hiding this comment.
Fixed. there was a dangling reference from ShowCollationFilterOpt, which is resolved in this diff
34d61fa to
2cc1e1d
Compare
For a high QPS application, the sqlparser generates a ton of garbage. ``` 622GB 26.33% 60.87% 880.50GB 37.28% vitess.io/vitess/go/vt/sqlparser.yyParse 212.57GB 9.00% 69.87% 258.49GB 10.94% vitess.io/vitess/go/vt/sqlparser.(*yyParserImpl).Parse ``` Most of this, per the heap profile is allocations of new `yyParserImpl` objects for every call to `yyParse`. This diff uses `sync.Pool` to mitigate the total allocation overhead. Modifies sql.y to fix a place where we took a reference to a stack variable rather than making an intermediate copy. Resolves vitessio#4174 Signed-off-by: Daniel Tahara <tahara@dropbox.com>
2cc1e1d to
3732474
Compare
For a high QPS application, the sqlparser generates a ton of garbage.
Most of this, per the heap profile is allocations of new
yyParserImplobjects for every call to
yyParse. This diff usessync.Pooltomitigate the total allocation overhead. Results:
Resolves: #4174
Signed-off-by: Daniel Tahara tahara@dropbox.com