Skip to content

- eliminate 2 allocations in EachKey()#223

Merged
buger merged 1 commit intobuger:masterfrom
Villenny:patch-3
Jan 22, 2021
Merged

- eliminate 2 allocations in EachKey()#223
buger merged 1 commit intobuger:masterfrom
Villenny:patch-3

Conversation

@Villenny
Copy link
Contributor

@Villenny Villenny commented Jan 4, 2021

Description:
Eliminate 2 allocations from EachKey()

Benchmark before change:

BenchmarkEachKey
BenchmarkEachKey-8
  308006	      3885 ns/op	     272 B/op	       3 allocs/op
PASS

Benchmark after change:

BenchmarkEachKey
BenchmarkEachKey-8
  333666	      3674 ns/op	       8 B/op	       1 allocs/op
PASS

Benchmarked with:

func BenchmarkEachKey(b *testing.B) {
	b.ResetTimer()
	for i := 0; i < b.N; i++ {
		var id string
		var bidId string
		var bidImpId string
		var bidPrice string
		var bidAdm string
		var cur string
		var foo string

		bodyBytes := stringHelper.StringAsBytes(BidResponse_Banner)

		jsonparser.EachKey(bodyBytes, func(idx int, value []byte, vt jsonparser.ValueType, err error) {
			switch idx {
			case 0:
				id = stringHelper.BytesAsString(value)
			case 1:
				bidId = stringHelper.BytesAsString(value)
			case 2:
				bidImpId = stringHelper.BytesAsString(value)
			case 3:
				bidPrice = stringHelper.BytesAsString(value)
			case 4:
				bidAdm = stringHelper.BytesAsString(value)
			case 5:
				cur = stringHelper.BytesAsString(value)
			case 6:
				foo = stringHelper.BytesAsString(value)
			}
		},
			[][]string{
				{"id"},
				{"seatbid", "[0]", "bid", "[0]", "id"},
				{"seatbid", "[0]", "bid", "[0]", "impid"},
				{"seatbid", "[0]", "bid", "[0]", "price"},
				{"seatbid", "[0]", "bid", "[0]", "adm"},
				{"cur"},
				{"foo"},
			}...,
		)

		if "9f3701ef-1d8a-4b67-a601-e9a1a2fbcb7c" != id ||
			"0" != bidId ||
			"1" != bidImpId ||
			"0.35258" != bidPrice ||
			`some \"html\" code\nnext line` != bidAdm ||
			"USD" != cur ||
			"" != foo {
			panic("ack")
		}
	}

}

const BidResponse_Banner = `
{
	"id":"9f3701ef-1d8a-4b67-a601-e9a1a2fbcb7c",
	"seatbid":[
	   {
		  "bid":[
			 {
				"id":"0",
				"adomain":[
				   "someurl.com",
				   "someotherurl.com"
				],
				"impid":"1",
				"price":0.35258,
				"adm":"some \"html\" code\nnext line",
				"w":300,
				"h":250
			 }
		  ]
	   }
	],
	"cur":"USD"
}
`

@buger
Copy link
Owner

buger commented Jan 8, 2021

Nice!

@xsandr
Copy link
Contributor

xsandr commented Jan 8, 2021

Looks good!
@Villenny could you please add the fix for pIdxFlags that you've mentioned? or do you prefer to do it separately?

@Villenny
Copy link
Contributor Author

Villenny commented Jan 8, 2021

Looks good!
@Villenny could you please add the fix for pIdxFlags that you've mentioned? or do you prefer to do it separately?

It wouldnt let me edit this pull request, its already done on my repo, in the patch-2 branch. But it wont let me add commits to this once the PR is open, so it will have to be another one.

@buger buger merged commit 275f2e8 into buger:master Jan 22, 2021
@buger
Copy link
Owner

buger commented Jan 22, 2021

@Villenny sorry for the delay, merged! Can't wait for the second PR :)

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