Optimization of fastJsonNode

[tzdybal]

fastJsonNode is strongly refactored and optimized. Code is simpler, consumes less memory and executes faster.

Changes:

• slices used instead of maps
• unified attrs and children containers into one (attrs), removed fastJsonAttr type
• (min-)heap used instead of sorting
• much simplified processing (because of single container in fastJsonNode)

Performance comparison - artificial test (committed to query/outputnode_test.go), times measured without profiler. Memory measured (for entire test execution, including all setup) with default profiler configuration (because -memprofilerate=1 was much too slow).

Branch	master	improve/encode_meomry	Improvement
Time:	172.283s	0.336s	> 512 x
Total memory"	43.65GB	283.30MB	> 157 x

In fact, memory improvement for encode function is much bigger - encode doesn't allocate memory. fastJsonNode creation is also much cheaper (in terms of memory).

---

This change is 

[manishrjain]

I don't totally understand it, but looks like a great change. I've got a few comments. Once @janardhan1993 approves this, feel free to submit.

---

Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, 6 unresolved discussions, some commit checks broke.

---

query/outputnode.go, line 292 at r1 (raw file):

func makeScalarNode(attr string, isChild bool, val []byte) *fastJsonNode {
	return &fastJsonNode{attr, 0, isChild, val, nil}

Please use the field names as well to qualify the variables.

---

query/outputnode.go, line 296 at r1 (raw file):

func makeNestedNode(attr string, isChild bool, val *fastJsonNode) *fastJsonNode {
	return &fastJsonNode{attr, 0, isChild, nil, []*fastJsonNode{val}}

Please use the field names to qualify the variables.

---

query/outputnode.go, line 427 at r1 (raw file):

	if fj.attrs.Len() > 0 {
		out.WriteRune('{')
		curr := heap.Pop(&fj.attrs).(*fastJsonNode)

s/curr/cur

---

query/outputnode.go, line 550 at r1 (raw file):

		// Merging with parent.
		parentSlice = merge(parentSlice, childSlice)
		//} else {

Remove?

---

query/outputnode_test.go, line 34 at r1 (raw file):

func makeFastJsonNode() *fastJsonNode {
	return &fastJsonNode{
	// uncoment for master branch

remove?

---

query/outputnode_test.go, line 41 at r1 (raw file):

func TestEncodeMemory(t *testing.T) {

Remove blank line.

---

Comments from Reviewable

[tzdybal]

Review status: all files reviewed at latest revision, 6 unresolved discussions.

---

query/outputnode.go, line 292 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Please use the field names as well to qualify the variables.

Done.

---

query/outputnode.go, line 296 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Please use the field names to qualify the variables.

Done.

---

query/outputnode.go, line 427 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

s/curr/cur

Done.

---

query/outputnode.go, line 550 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Remove?

Done.

---

query/outputnode_test.go, line 34 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

remove?

Done.

---

query/outputnode_test.go, line 41 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Remove blank line.

Done.

---

Comments from Reviewable

[janardhan1993]

---

Reviewed 2 of 3 files at r2.
Review status: 1 of 2 files reviewed at latest revision, 7 unresolved discussions.

---

query/outputnode.go, line 350 at r2 (raw file):

func (fj *fastJsonNode) SetUID(uid uint64, attr string) {
	fj.attrs = append(fj.attrs, makeScalarNode(attr, false, []byte(fmt.Sprintf("\"%#x\"", uid))))

I am not that familiar with this code, but if addlistchild, addmapchild or setuid are called twice then we would have the same output twice ?
If we are running it in debug mode and user has also requested for uid, setUid would be called twice.

---

Comments from Reviewable

[tzdybal]

Review status: 1 of 4 files reviewed at latest revision, 7 unresolved discussions.

---

query/outputnode.go, line 350 at r2 (raw file):

Previously, janardhan1993 (Janardhan Reddy) wrote…

I am not that familiar with this code, but if addlistchild, addmapchild or setuid are called twice then we would have the same output twice ?
If we are running it in debug mode and user has also requested for uid, setUid would be called twice.

Special handling in SetUID function added (with test supporting this behaviour). Case where user asks for same predicate many times are handled earlier during the processing.

---

Comments from Reviewable