Use jsoniter to encode object #54289

ggaaooppeenngg · 2017-10-20T09:50:05Z

standard encoding/json has performance issue, use jsoniter
can help to reduce half of the encoding time.

Signed-off-by: Peng Gao peng.gao.dut@gmail.com

What this PR does / why we need it:
This PR change encoding/json to jsoniter, from the profiling it reduce the half of processing time.

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

Release note:

NONE

/sig api-machinery

standard encoding/json has performance issue,use jsoniter can help to reduce half of the encoding time. Signed-off-by: Peng Gao <peng.gao.dut@gmail.com>

k8s-ci-robot · 2017-10-20T09:50:12Z

Hi @ggaaooppeenngg. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

k8s-github-robot · 2017-10-20T09:50:38Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ggaaooppeenngg
We suggest the following additional approver: caesarxuchao

Assign the PR to them by writing /assign @caesarxuchao in a comment when ready.

No associated issue. Update pull-request body to add a reference to an issue, or get approval with /approve no-issue

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

staging/src/k8s.io/apimachinery/pkg/OWNERS

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

ggaaooppeenngg · 2017-10-20T09:53:02Z

/assign @caesarxuchao

mkumatag · 2017-10-20T10:34:22Z

I just see only this entry in the whole file using encoding/json. I think we can use jsoniter to remove the complete dependency of encoding/json in this file.

mkumatag · 2017-10-20T10:34:36Z

/cc @thockin

ggaaooppeenngg · 2017-10-20T17:58:23Z

I think it's ok to remove encoding/json dependency in this file, json.Number underling type is string.

mkumatag · 2017-10-20T18:29:52Z

/ok-to-test

liggitt · 2017-10-20T22:59:13Z

it doesn't appear that jsoniter normalizes key order:

--- FAIL: TestDeepCopyApiObjects (3.32s)
	copy_test.go:74: {"name":"iÃ¥Ç©Ã�æ¹¶&Å¬;ç¨«nTå’œpç‘�å�®É‡","generateName":"cÃ“Ã¤É‰ç¤ˆÃ—Ä’Pç¨Šé©�É”","selfLink":";c","uid":"Å³ÉƒÇ�é«€_Kè€‹é¡¬éœ§(-+","resourceVersion":"9655338397498521902","creationTimestamp":null,"Spec":{"AccessModes":["Ä¿vé‡‰ç�»`SçŸ˜èƒ„"],"Selector":{"matchLabels":{}},"Resources":{"Limits":{"
		
		A: storage":"28","cpu":"88","memory":"63"},"Requests":{"cpu":"88","memory":"63","storage":"28"}},"VolumeName":"å§¥HÊ‹éŸ›åª æ±›6é…™T'","StorageClassName":""},"Status":{"Phase":"Lost","AccessModes":["Æ‰#å¨ˆç ¿Vè§»zå‘•Â¥Ã¥JÅ¡AIÆŒpÅ¡é‡‘Å›"],"Capacity":null,"Conditions":[]}}
		
		B: cpu":"88","memory":"63","storage":"28"},"Requests":{"memory":"63","storage":"28","cpu":"88"}},"VolumeName":"å§¥HÊ‹éŸ›åª æ±›6é…™T'","StorageClassName":""},"Status":{"Phase":"Lost","AccessModes":["Æ‰#å¨ˆç ¿Vè§»zå‘•Â¥Ã¥JÅ¡AIÆŒpÅ¡é‡‘Å›"],"Capacity":null,"Conditions":[]}}

stable order is required

liggitt · 2017-10-20T23:00:13Z

@thockin did you exercise encoding in jsoniter as thoroughly as decoding?

Signed-off-by: Peng Gao <peng.gao.dut@gmail.com>

thockin · 2017-10-23T05:50:20Z

staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/json.go

@@ -216,7 +215,7 @@ func (s *Serializer) Encode(obj runtime.Object, w io.Writer) error {
 		_, err = w.Write(data)
 		return err
 	}
-	encoder := json.NewEncoder(w)
+	encoder := jsoniter.ConfigCompatibleWithStandardLibrary.NewEncoder(w)


Everything looks OK except this. Doesn't this have a negative impact of performance? It did when I measured it.

I offered a bench commit, on my machine it's

pkg: k8s.io/kubernetes/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json BenchmarkJsonEncoding-4 3000000 569 ns/op BenchmarkJsoniterEncoding-4 5000000 316 ns/op

And from my cluster testing, the time share decreases from 60%s to 25%. How did you measure it?

The benchmark doesn't use data that is similar to actual objects; can you get e.g. a big pod spec or something and run again?

@lavalamp I pushed a commit, the result was not better, but just 20% improved.

pkg: k8s.io/kubernetes/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json BenchmarkJsonEncoding-4 100000 13080 ns/op BenchmarkJsoniterEncoding-4 200000 11158 ns/op PASS

Signed-off-by: Peng Gao <peng.gao.dut@gmail.com>

ggaaooppeenngg

I made up a large pod for benchmark.

ggaaooppeenngg · 2017-10-24T15:51:46Z

staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json/json.go

@@ -216,7 +215,7 @@ func (s *Serializer) Encode(obj runtime.Object, w io.Writer) error {
 		_, err = w.Write(data)
 		return err
 	}
-	encoder := json.NewEncoder(w)
+	encoder := jsoniter.ConfigCompatibleWithStandardLibrary.NewEncoder(w)


@lavalamp I pushed a commit, the result was not better, but just 20% improved.

pkg: k8s.io/kubernetes/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json BenchmarkJsonEncoding-4 100000 13080 ns/op BenchmarkJsoniterEncoding-4 200000 11158 ns/op PASS

Signed-off-by: Peng Gao <peng.gao.dut@gmail.com>

ggaaooppeenngg · 2017-10-25T16:05:34Z

/retest

ggaaooppeenngg · 2017-10-26T05:35:09Z

/retest

liggitt · 2017-10-26T05:42:33Z

consistent failure:

--- FAIL: TestGetJobFromTemplate (0.00s)
	utils_test.go:96: Wrong length for created-by annotation, expected 160 got 159
	utils_test.go:99: Wrong value for created-by annotation, expected {"kind":"SerializedReference","apiVersion":"v1","reference":{"kind":"CronJob","namespace":"snazzycats","name":"mycronjob","uid":"1a2b3c","apiVersion":"batch"}}
		 got {"kind":"SerializedReference","apiVersion":"v1","reference":{"kind":"CronJob","namespace":"snazzycats","name":"mycronjob","uid":"1a2b3c","apiVersion":"batch"}}

Signed-off-by: Peng Gao <peng.gao.dut@gmail.com>

k8s-ci-robot · 2017-10-27T04:45:58Z

@ggaaooppeenngg: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kubernetes-unit	`ec29346`	link	`/test pull-kubernetes-unit`
pull-kubernetes-e2e-kops-aws	`ec29346`	link	`/test pull-kubernetes-e2e-kops-aws`
pull-kubernetes-e2e-gce	`ec29346`	link	`/test pull-kubernetes-e2e-gce`
pull-kubernetes-bazel-test	`ec29346`	link	`/test pull-kubernetes-bazel-test`
pull-kubernetes-verify	`ec29346`	link	`/test pull-kubernetes-verify`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

ggaaooppeenngg · 2017-10-29T15:51:58Z

In order to make json-iter to be compatible with standard library, it needs some work, wait for the lib stable then open it again.

Use jsoniter to encode object

fea6e69

standard encoding/json has performance issue,use jsoniter can help to reduce half of the encoding time. Signed-off-by: Peng Gao <peng.gao.dut@gmail.com>

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 20, 2017

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 20, 2017

k8s-github-robot assigned tallclair Oct 20, 2017

k8s-github-robot assigned caesarxuchao Oct 20, 2017

k8s-ci-robot requested a review from thockin October 20, 2017 10:34

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Oct 20, 2017

ggaaooppeenngg added 2 commits October 21, 2017 10:33

Remove encoding/json dependency in file

32f7cd8

Signed-off-by: Peng Gao <peng.gao.dut@gmail.com>

Set encoding mode to standard compatible

4970446

Signed-off-by: Peng Gao <peng.gao.dut@gmail.com>

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 21, 2017

thockin reviewed Oct 23, 2017

View reviewed changes

Offer a small benchmark, result is 40% improved

39f089f

Signed-off-by: Peng Gao <peng.gao.dut@gmail.com>

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 23, 2017

ggaaooppeenngg commented Oct 24, 2017

View reviewed changes

Use large pod for encoding, with 20% improvement

f8c34c5

Signed-off-by: Peng Gao <peng.gao.dut@gmail.com>

ggaaooppeenngg force-pushed the api-json-encoding branch from 2661810 to f8c34c5 Compare October 24, 2017 16:14

ggaaooppeenngg mentioned this pull request Oct 26, 2017

Fix standard compatiblility json-iterator/go#188

Merged

update json-iter

b722cf9

Signed-off-by: Peng Gao <peng.gao.dut@gmail.com>

ggaaooppeenngg mentioned this pull request Oct 26, 2017

Fix standard compatiblility json-iterator/go#189

Merged

ggaaooppeenngg added 2 commits October 27, 2017 10:28

update json-iter

846b4b9

Signed-off-by: Peng Gao <peng.gao.dut@gmail.com>

update debug info

ec29346

Signed-off-by: Peng Gao <peng.gao.dut@gmail.com>

ggaaooppeenngg closed this Oct 29, 2017

ggaaooppeenngg deleted the api-json-encoding branch October 29, 2017 15:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use jsoniter to encode object #54289

Use jsoniter to encode object #54289

ggaaooppeenngg commented Oct 20, 2017 •

edited

k8s-ci-robot commented Oct 20, 2017

k8s-github-robot commented Oct 20, 2017

ggaaooppeenngg commented Oct 20, 2017

mkumatag commented Oct 20, 2017

mkumatag commented Oct 20, 2017

ggaaooppeenngg commented Oct 20, 2017

mkumatag commented Oct 20, 2017

liggitt commented Oct 20, 2017

liggitt commented Oct 20, 2017

thockin Oct 23, 2017

ggaaooppeenngg Oct 23, 2017 •

edited

lavalamp Oct 23, 2017

ggaaooppeenngg Oct 24, 2017

ggaaooppeenngg left a comment

ggaaooppeenngg Oct 24, 2017

ggaaooppeenngg commented Oct 25, 2017 •

edited

ggaaooppeenngg commented Oct 26, 2017

liggitt commented Oct 26, 2017

k8s-ci-robot commented Oct 27, 2017

ggaaooppeenngg commented Oct 29, 2017

Use jsoniter to encode object #54289

Use jsoniter to encode object #54289

Conversation

ggaaooppeenngg commented Oct 20, 2017 • edited

k8s-ci-robot commented Oct 20, 2017

k8s-github-robot commented Oct 20, 2017

ggaaooppeenngg commented Oct 20, 2017

mkumatag commented Oct 20, 2017

mkumatag commented Oct 20, 2017

ggaaooppeenngg commented Oct 20, 2017

mkumatag commented Oct 20, 2017

liggitt commented Oct 20, 2017

liggitt commented Oct 20, 2017

thockin Oct 23, 2017

Choose a reason for hiding this comment

ggaaooppeenngg Oct 23, 2017 • edited

Choose a reason for hiding this comment

lavalamp Oct 23, 2017

Choose a reason for hiding this comment

ggaaooppeenngg Oct 24, 2017

Choose a reason for hiding this comment

ggaaooppeenngg left a comment

Choose a reason for hiding this comment

ggaaooppeenngg Oct 24, 2017

Choose a reason for hiding this comment

ggaaooppeenngg commented Oct 25, 2017 • edited

ggaaooppeenngg commented Oct 26, 2017

liggitt commented Oct 26, 2017

k8s-ci-robot commented Oct 27, 2017

ggaaooppeenngg commented Oct 29, 2017

ggaaooppeenngg commented Oct 20, 2017 •

edited

ggaaooppeenngg Oct 23, 2017 •

edited

ggaaooppeenngg commented Oct 25, 2017 •

edited