Skip to content
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

[BUG] [Go] MarshalJSON is extremely slow. (Performance Issue) #16948

Closed
5 of 6 tasks
kokoichi206 opened this issue Oct 31, 2023 · 5 comments
Closed
5 of 6 tasks

[BUG] [Go] MarshalJSON is extremely slow. (Performance Issue) #16948

kokoichi206 opened this issue Oct 31, 2023 · 5 comments

Comments

@kokoichi206
Copy link
Contributor

kokoichi206 commented Oct 31, 2023

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • Have you tested with the latest master to confirm the issue still exists?
  • Have you searched for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

It might not be appropriate to raise this as a bug report.

json.Marshal of the generated struct was extremely slow (5-10x).

I guess that the cause lies in the part of the MarshalJSON method for the struct generation, where it's converting to a map of string and interface.
I couldn't understand the reason for processing every field in this manner.

I'd appreciate it if you could explain the reason for this kind of generation and, if possible, let me know how to avoid it during the generation process.

openapi-generator version

openapi-generator-cli:v7.0.0
(v5.1.1 also reproduced)

OpenAPI declaration file content or url

I used the petstore example from OpenAPI-Specification

Generation Details

I used docker of the cli.

docker run --rm -v $(shell pwd):/project openapitools/openapi-generator-cli:v7.0.0 \
	generate -i /project/openapi.yaml -g go \
	--additional-properties=packageName=component -o /project/gen/component -p enumClassPrefix=true

.openapi-generator-ignore

api_*
client.go
configuration.go
.travis.yml

README.md
git_push.sh

go.mod
go.sum

api/
docs/
Steps to reproduce
  1. Generate code for 2 packages using the "Generation Details" docker and petstore example.
  2. Leave one generated as is, with the package name "component_method".
  3. Rename the other one to the package name "component" and remove MarshalJSON and UnmarshalJSON from the generated code.
  4. Do Benchmark tests for json.Marshal and json.Unmarshal.
  5. json.Marshal of "component_method" is 6x slower than "component"

test results

$ go test -bench .
goos: darwin
goarch: arm64
pkg: benchmark
BenchmarkJsonUnMarshal-8                 1225986               987.5 ns/op
BenchmarkJsonUnMarshalMethod-8           1229346               977.5 ns/op
BenchmarkJsonMarshal-8                   7769258               153.6 ns/op
BenchmarkJsonMarshalMethod-8             1256686               956.0 ns/op
PASS
ok      benchmark       8.116s

The benchmark test code

package main

import (
	"benchmark/gen/component"
	"benchmark/gen/component_method"
	"encoding/json"
	"os"
	"testing"
)

func BenchmarkJsonUnMarshal(b *testing.B) {
	bytes, _ := os.ReadFile("res.json")

	b.ResetTimer()
	for i := 0; i < b.N; i++ {
		var res component.Pet
		json.Unmarshal(bytes, &res)
	}
}

func BenchmarkJsonUnMarshalMethod(b *testing.B) {
	bytes, _ := os.ReadFile("res.json")

	b.ResetTimer()
	for i := 0; i < b.N; i++ {
		var res component_method.Pet
		json.Unmarshal(bytes, &res)
	}
}

func BenchmarkJsonMarshal(b *testing.B) {
	bytes, _ := os.ReadFile("res.json")
	var res component.Pet
	json.Unmarshal(bytes, &res)

	b.ResetTimer()
	for i := 0; i < b.N; i++ {
		_, _ = json.Marshal(res)
	}
}

func BenchmarkJsonMarshalMethod(b *testing.B) {
	bytes, _ := os.ReadFile("res.json")
	var res component_method.Pet
	json.Unmarshal(bytes, &res)

	b.ResetTimer()
	for i := 0; i < b.N; i++ {
		_, _ = json.Marshal(res)
	}
}

res.json

{
    "id": 111,
    "name": "Test name 111",
    "tag": [
        "test tag",
        "kawaii"
    ]
}

supplement

  • This behavior also reproduced for larger json, struct
  • json.Unmarshal is not slow
Related issues/PRs
Suggest a fix
@wing328
Copy link
Member

wing328 commented Nov 1, 2023

@kokoichi206 thanks for brining it up and the benchmark result.

If I remember correctly, the customized MarshalJSON method was added to meet the requirement of different use cases (e.g. oneOf/anyOf schema in properties, additional properties, nullable properties, etc)

For your use cases, if these are not needed, what about adding an option to skip generating the MarshalJSON method? Would that help?

@wing328
Copy link
Member

wing328 commented Nov 1, 2023

FYI @antihax (2017/11) @grokify (2018/07) @kemokemo (2018/09) @jirikuncar (2021/01) @ph4r5h4d (2021/04) @lwj5 (2023/04)

@wing328 wing328 added go Pull requests that update Go code Client: Go and removed go Pull requests that update Go code labels Nov 1, 2023
@wing328 wing328 added this to the 7.1.0 milestone Nov 1, 2023
@kokoichi206
Copy link
Contributor Author

@wing328
Thank you for your reply.

Having an option to skip generating the MarshalJSON method would be a welcome addition for our use case.
If it's acceptable, I would also be interested in trying to implement this feature myself.

@wing328
Copy link
Member

wing328 commented Nov 1, 2023

If it's acceptable, I would also be interested in trying to implement this feature myself.

yes please. let me know if you need help with the PR.

wing328 pushed a commit that referenced this issue Nov 2, 2023
…16962)

* [add] additionalProperties about whether generating MarshalJSON (#16948)

* [change] key from skipGeneratingMarshalJSON to generateMarshalJSON (#16948)

* [test] modify unit tests (#16948)

* [fix] default value (#16948)

* [update] samples (#16948)

* [fix] document (#16948)
@wing328 wing328 modified the milestones: 7.1.0, 7.2.0 Nov 13, 2023
@wing328
Copy link
Member

wing328 commented Nov 13, 2023

closing this as the PR has been merged.

happy to reopen if needed.

@wing328 wing328 closed this as completed Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants