Skip to content

feat: Json type optimization #2006

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

Merged
merged 12 commits into from
Sep 12, 2024
Merged

feat: Json type optimization #2006

merged 12 commits into from
Sep 12, 2024

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

renderer := column.NewRenderer()

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
@@ -302,6 +302,154 @@ func TestRenderer_Path_FormData_Full(t *testing.T) {
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
13 checks passed
@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