Skip to content

feat: Json type optimization#2006

Merged
jachym-tousek-keboola merged 12 commits intomainfrom
jt-psgo-734-fastjson
Sep 12, 2024
Merged

feat: Json type optimization#2006
jachym-tousek-keboola merged 12 commits intomainfrom
jt-psgo-734-fastjson

Conversation

@jachym-tousek-keboola
Copy link
Member

Jira: https://keboola.atlassian.net/browse/PSGO-734

Changes:

  • Optimized column type json using fastjson
  • Added more tests to ensure no behavior changes
  • Improved behavior for form data

Benchmarks

The BenchmarkColumn_Path benchmark is the relevant one here.

Old implementation:

/code > go test ./internal/pkg/service/stream/mapping/table/column/renderer_benchmark_test.go -bench=. -benchmem
goos: linux
goarch: amd64
cpu: 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz
BenchmarkColumn_Path-8                    161614              8795 ns/op            5131 B/op        105 allocs/op
BenchmarkColumn_Template_Jsonnet-8         61544             22324 ns/op           13993 B/op        223 allocs/op
BenchmarkColumn_UUID-8                    960573              1052 ns/op             568 B/op          6 allocs/op
PASS
ok      command-line-arguments  4.101s

New implementation with fastjson:

/code > go test ./internal/pkg/service/stream/mapping/table/column/renderer_benchmark_test.go -bench=. -benchmem
goos: linux
goarch: amd64
cpu: 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz
BenchmarkColumn_Path-8                    492908              2574 ns/op            1908 B/op         26 allocs/op
BenchmarkColumn_Template_Jsonnet-8         63018             20630 ns/op           13973 B/op        223 allocs/op
BenchmarkColumn_UUID-8                   1000000              1056 ns/op             568 B/op          6 allocs/op
PASS
ok      command-line-arguments  3.864s


for i := 0; i < b.N; i++ {
// reqCtx needs to be created separately for each request, otherwise the parsed json is cached
reqCtx := recordctx.FromHTTP(time.Now(), &http.Request{Header: header, Body: io.NopCloser(strings.NewReader(body))})
Copy link
Member Author

Choose a reason for hiding this comment

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

reqCtx caches the parsed json in case it is needed for other columns so we need to create a new one for each CSVValue call, otherwise json parsing is cached, making the benchmark irrelevant.

@jachym-tousek-keboola jachym-tousek-keboola changed the title tests: Add missing tests feat: Json type optimization Sep 10, 2024
assert.Equal(t, `{"key1":"bar1","key2":["bar2","bar3"]}`, val)
}

func TestRenderer_Path_FormData_Scalar(t *testing.T) {
Copy link
Member Author

@jachym-tousek-keboola jachym-tousek-keboola Sep 10, 2024

Choose a reason for hiding this comment

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

Extra tests for application/x-www-form-urlencoded since the original renderer implementation is no longer covered by json tests + the results changed because of ParseQuery improvements.

@jachym-tousek-keboola jachym-tousek-keboola marked this pull request as ready for review September 10, 2024 12:04
Copy link
Contributor

@michaljurecko michaljurecko left a comment

Choose a reason for hiding this comment

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

LGTM

@jachym-tousek-keboola jachym-tousek-keboola force-pushed the jt-psgo-734-fastjson branch 2 times, most recently from 87f54d9 to 514f841 Compare September 12, 2024 08:37
@jachym-tousek-keboola jachym-tousek-keboola force-pushed the jt-psgo-734-fastjson branch 3 times, most recently from 67159a4 to b158a2e Compare September 12, 2024 09:33
@jachym-tousek-keboola jachym-tousek-keboola merged commit 8965049 into main Sep 12, 2024
@jachym-tousek-keboola jachym-tousek-keboola deleted the jt-psgo-734-fastjson branch September 12, 2024 11:12
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.

3 participants