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

Perf: remove reliance on in-box JSON marshaller #19356

Open
jhendrixMSFT opened this issue Oct 13, 2022 · 2 comments
Open

Perf: remove reliance on in-box JSON marshaller #19356

jhendrixMSFT opened this issue Oct 13, 2022 · 2 comments
Assignees
Labels
auto-close-exempt Prevents the auto-close from closing based on max lifetime CodeGen Issues that relate to code generation pillar-performance The issue is related to performance, one of our core engineering pillars.
Milestone

Comments

@jhendrixMSFT
Copy link
Member

jhendrixMSFT commented Oct 13, 2022

Our reliance on the standard library's JSON marshaller/unmarshaller is unnecessary (and expensive) as we have all the type information available during code generation. Consider the below benchmarks.

The second benchmark (BenchmarkPrototypeMarshaller) uses a custom JSON marshaller that builds the JSON string with a strings.Builder. It starts with a 512-byte buffer (more didn't improve things). However, this value might need to be tuned for optimal performance.

The win is very clear and is worth investigating further.

go test -benchmem -bench .
goos: windows
goarch: amd64
pkg: armdataboxedge
cpu: AMD Ryzen 7 2700X Eight-Core Processor
BenchmarkCurrentMarshaller-4               87075             12604 ns/op            3121 B/op         40 allocs/op
BenchmarkPrototypeMarshaller-4            546716              2150 ns/op            1920 B/op          7 allocs/op
PASS
ok      armdataboxedge  2.839s
var addon = armdataboxedge.Addon{
	Kind: to.Ptr(armdataboxedge.AddonTypeArcForKubernetes),
	ID:   to.Ptr("/some/kind/of/resource/that/should/be/a/long/string/longer/than/this/most/likely"),
	Name: to.Ptr("FooBarBaz"),
	SystemData: &armdataboxedge.SystemData{
		CreatedAt:          to.Ptr(time.Now()),
		CreatedBy:          to.Ptr("jhendrix"),
		CreatedByType:      to.Ptr(armdataboxedge.CreatedByTypeUser),
		LastModifiedAt:     to.Ptr(time.Now()),
		LastModifiedBy:     to.Ptr("jhendrix"),
		LastModifiedByType: to.Ptr(armdataboxedge.CreatedByTypeUser),
	},
	Type: to.Ptr("something/something"),
}

func BenchmarkCurrentMarshaller(b *testing.B) {
	for i := 0; i < b.N; i++ {
		if _, err := json.Marshal(addon); err != nil {
			b.Fatal(err)
		}
	}
}

func BenchmarkPrototypeMarshaller(b *testing.B) {
	for i := 0; i < b.N; i++ {
		if _, err := addon.MarshalJSON2(); err != nil {
			b.Fatal(err)
		}
	}
}
@jhendrixMSFT jhendrixMSFT added CodeGen Issues that relate to code generation pillar-performance The issue is related to performance, one of our core engineering pillars. labels Oct 13, 2022
@jhendrixMSFT jhendrixMSFT self-assigned this Oct 13, 2022
@jhendrixMSFT jhendrixMSFT changed the title Perf: remove reliance on in-box JSON marshaller/unmarshaller Perf: remove reliance on in-box JSON marshaller Oct 13, 2022
@jhendrixMSFT
Copy link
Member Author

jhendrixMSFT commented Oct 13, 2022

Unfortunately, json.Decoder has some perf problems which prevents us from building performant, custom unmarshallers on top of it (see golang/go#40128). For now, we'll just fix up the marshallers.

BenchmarkCurrentUnmarshaller-4             46093             26880 ns/op            4857 B/op         87 allocs/op
BenchmarkPrototypeUnmarshaller-4           44324             25808 ns/op            7200 B/op        286 allocs/op

Copy link

Hi @jhendrixMSFT, we deeply appreciate your input into this project. Regrettably, this issue has remained unresolved for over 2 years and inactive for 30 days, leading us to the decision to close it. We've implemented this policy to maintain the relevance of our issue queue and facilitate easier navigation for new contributors. If you still believe this topic requires attention, please feel free to create a new issue, referencing this one. Thank you for your understanding and ongoing support.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 14, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Oct 14, 2024
@jhendrixMSFT jhendrixMSFT reopened this Oct 15, 2024
@jhendrixMSFT jhendrixMSFT added the auto-close-exempt Prevents the auto-close from closing based on max lifetime label Oct 15, 2024
@RickWinter RickWinter added this to the Backlog milestone Nov 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-close-exempt Prevents the auto-close from closing based on max lifetime CodeGen Issues that relate to code generation pillar-performance The issue is related to performance, one of our core engineering pillars.
Projects
None yet
Development

No branches or pull requests

2 participants