Repository: genjidb/genji
Pull request #270 (2020-10-17 13:59:17)

Slightly more efficient conversion from JSON to document
This PR is a small, low hanging, premature optimization.
It changes the way the `document.NewFromJSON` function works by lazily decoding the json data and delegating error handling to the `Iterate` and `GetByField` methods.
This takes advantage of the `jsonparser` library and prevents from duplicating the entire json object in memory before storing it.

It's 50% faster for small objects and about 20% for large objects, and 80% more memory efficient. It might be useful later for quick stream insertion.

```
name               old time/op    new time/op    delta
JSONToDocument-12    9.17µs ± 0%    7.86µs ± 0%   ~     (p=1.000 n=1+1)

name               old alloc/op   new alloc/op   delta
JSONToDocument-12    5.65kB ± 0%    3.10kB ± 0%   ~     (p=1.000 n=1+1)

name               old allocs/op  new allocs/op  delta
JSONToDocument-12       106 ± 0%       100 ± 0%   ~     (p=1.000 n=1+1)
```

codecov-io: # [Codecov](https://codecov.io/gh/genjidb/genji/pull/270?src=pr&el=h1) Report
> Merging [#270](https://codecov.io/gh/genjidb/genji/pull/270?src=pr&el=desc) into [master](https://codecov.io/gh/genjidb/genji/commit/8fc7000fe8c8f82ee6d1b2b38f6f3498e5e7f76f?el=desc) will **increase** coverage by `0.05%`.
> The diff coverage is `81.25%`.

[![Impacted file tree graph](https://codecov.io/gh/genjidb/genji/pull/270/graphs/tree.svg?width=650&height=150&src=pr&token=nwR1GpvtyH)](https://codecov.io/gh/genjidb/genji/pull/270?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #270      +/-   ##
==========================================
+ Coverage   61.35%   61.41%   +0.05%     
==========================================
  Files          68       68              
  Lines        6345     6354       +9     
==========================================
+ Hits         3893     3902       +9     
  Misses       1936     1936              
  Partials      516      516              
```


| [Impacted Files](https://codecov.io/gh/genjidb/genji/pull/270?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [document/create.go](https://codecov.io/gh/genjidb/genji/pull/270/diff?src=pr&el=tree#diff-ZG9jdW1lbnQvY3JlYXRlLmdv) | `68.27% <72.72%> (+0.15%)` | :arrow_up: |
| [database/table.go](https://codecov.io/gh/genjidb/genji/pull/270/diff?src=pr&el=tree#diff-ZGF0YWJhc2UvdGFibGUuZ28=) | `57.25% <100.00%> (ø)` | |
| [document/array.go](https://codecov.io/gh/genjidb/genji/pull/270/diff?src=pr&el=tree#diff-ZG9jdW1lbnQvYXJyYXkuZ28=) | `73.33% <100.00%> (+0.45%)` | :arrow_up: |
| [document/document.go](https://codecov.io/gh/genjidb/genji/pull/270/diff?src=pr&el=tree#diff-ZG9jdW1lbnQvZG9jdW1lbnQuZ28=) | `75.93% <100.00%> (+1.06%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/genjidb/genji/pull/270?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/genjidb/genji/pull/270?src=pr&el=footer). Last update [8fc7000...7f228c9](https://codecov.io/gh/genjidb/genji/pull/270?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

tie: It looks like you din’t set the `-count` flag when running benchmarks (`p=1.000 n=1+1`).

asdine: 10 times 👍🏼 
```
name               old time/op    new time/op    delta
JSONToDocument-12    8.39µs ± 3%    7.06µs ± 2%  -15.82%  (p=0.000 n=10+9)

name               old alloc/op   new alloc/op   delta
JSONToDocument-12    5.65kB ± 0%    3.10kB ± 0%  -45.04%  (p=0.000 n=10+10)

name               old allocs/op  new allocs/op  delta
JSONToDocument-12       106 ± 0%       100 ± 0%   -5.66%  (p=0.000 n=10+10)
```

Patch:
diff --git a/database/table.go b/database/table.go
index 515a5ac0..2552d0a9 100644
--- a/database/table.go
+++ b/database/table.go
@@ -514,7 +514,7 @@ func validateConstraint(d document.Document, c *FieldConstraint) error {
 		}
 	case document.ArrayValue:
 		// if it's an array, we can assume it's a ValueBuffer
-		buf := parent.V.(*document.ValueBuffer)
+		buf := parent.V.(document.ValueBuffer)
 
 		frag := c.Path[len(c.Path)-1]
 		if frag.FieldName != "" {
diff --git a/document/array.go b/document/array.go
index 58bf0683..b1ee610b 100644
--- a/document/array.go
+++ b/document/array.go
@@ -68,6 +68,11 @@ type ValueBuffer []Value
 
 // NewValueBuffer creates a buffer of values.
 func NewValueBuffer(values ...Value) ValueBuffer {
+	if len(values) == 0 {
+		// If called with no values return a non-nil slice.
+		return ValueBuffer{}
+	}
+
 	return ValueBuffer(values)
 }
 
@@ -141,7 +146,7 @@ func (vb *ValueBuffer) Copy(a Array) error {
 				return err
 			}
 
-			err = vb.Replace(i, NewArrayValue(&buf))
+			err = vb.Replace(i, NewArrayValue(buf))
 			if err != nil {
 				return err
 			}
diff --git a/document/create.go b/document/create.go
index ce4014b9..07a3e1e5 100644
--- a/document/create.go
+++ b/document/create.go
@@ -9,16 +9,43 @@ import (
 	"reflect"
 	"strings"
 	"time"
+
+	"github.com/buger/jsonparser"
 )
 
-// NewFromJSON creates a document from a JSON object.
-func NewFromJSON(data []byte) (Document, error) {
-	var fb FieldBuffer
-	err := fb.UnmarshalJSON(data)
+// NewFromJSON creates a document from raw JSON data.
+// The returned document will lazily decode the data.
+// If data is not a valid json object, calls to Iterate or GetByField will
+// return an error.
+func NewFromJSON(data []byte) Document {
+	return &jsonEncodedDocument{data}
+}
+
+type jsonEncodedDocument struct {
+	data []byte
+}
+
+func (j jsonEncodedDocument) Iterate(fn func(field string, value Value) error) error {
+	return jsonparser.ObjectEach(j.data, func(key, value []byte, dataType jsonparser.ValueType, offset int) error {
+		v, err := parseJSONValue(dataType, value)
+		if err != nil {
+			return err
+		}
+
+		return fn(string(key), v)
+	})
+}
+
+func (j jsonEncodedDocument) GetByField(field string) (Value, error) {
+	v, dt, _, err := jsonparser.Get(j.data, field)
+	if dt == jsonparser.NotExist {
+		return Value{}, ErrFieldNotFound
+	}
 	if err != nil {
-		return nil, err
+		return Value{}, err
 	}
-	return &fb, nil
+
+	return parseJSONValue(dt, v)
 }
 
 // NewFromMap creates a document from a map.
diff --git a/document/document.go b/document/document.go
index 648a32d4..7b9c930e 100644
--- a/document/document.go
+++ b/document/document.go
@@ -290,7 +290,7 @@ func (fb *FieldBuffer) Copy(d Document) error {
 				return err
 			}
 
-			fb.fields[i].Value = NewArrayValue(&buf)
+			fb.fields[i].Value = NewArrayValue(buf)
 		}
 	}
 


Test patch:
diff --git a/document/create_test.go b/document/create_test.go
new file mode 100644
index 00000000..d86cf2a8
--- /dev/null
+++ b/document/create_test.go
@@ -0,0 +1,86 @@
+package document_test
+
+import (
+	"testing"
+
+	"github.com/genjidb/genji/document"
+	"github.com/stretchr/testify/require"
+)
+
+func TestNewFromJSON(t *testing.T) {
+	tests := []struct {
+		name     string
+		data     string
+		expected *document.FieldBuffer
+		fails    bool
+	}{
+		{"empty object", "{}", document.NewFieldBuffer(), false},
+		{"empty object, missing closing bracket", "{", nil, true},
+		{"classic object", `{"a": 1, "b": true, "c": "hello", "d": [1, 2, 3], "e": {"f": "g"}}`,
+			document.NewFieldBuffer().
+				Add("a", document.NewIntegerValue(1)).
+				Add("b", document.NewBoolValue(true)).
+				Add("c", document.NewTextValue("hello")).
+				Add("d", document.NewArrayValue(document.NewValueBuffer().
+					Append(document.NewIntegerValue(1)).
+					Append(document.NewIntegerValue(2)).
+					Append(document.NewIntegerValue(3)))).
+				Add("e", document.NewDocumentValue(document.NewFieldBuffer().Add("f", document.NewTextValue("g")))),
+			false},
+		{"string values", `{"a": "hello ciao"}`, document.NewFieldBuffer().Add("a", document.NewTextValue("hello ciao")), false},
+		{"+integer values", `{"a": 1000}`, document.NewFieldBuffer().Add("a", document.NewIntegerValue(1000)), false},
+		{"-integer values", `{"a": -1000}`, document.NewFieldBuffer().Add("a", document.NewIntegerValue(-1000)), false},
+		{"+float values", `{"a": 10000000000.0}`, document.NewFieldBuffer().Add("a", document.NewDoubleValue(10000000000)), false},
+		{"-float values", `{"a": -10000000000.0}`, document.NewFieldBuffer().Add("a", document.NewDoubleValue(-10000000000)), false},
+		{"bool values", `{"a": true, "b": false}`, document.NewFieldBuffer().Add("a", document.NewBoolValue(true)).Add("b", document.NewBoolValue(false)), false},
+		{"empty arrays", `{"a": []}`, document.NewFieldBuffer().Add("a", document.NewArrayValue(document.NewValueBuffer())), false},
+		{"nested arrays", `{"a": [[1,  2]]}`, document.NewFieldBuffer().
+			Add("a", document.NewArrayValue(
+				document.NewValueBuffer().
+					Append(document.NewArrayValue(
+						document.NewValueBuffer().
+							Append(document.NewIntegerValue(1)).
+							Append(document.NewIntegerValue(2)))))), false},
+		{"missing comma", `{"a": 1 "b": 2}`, nil, true},
+		{"missing closing brackets", `{"a": 1, "b": 2`, nil, true},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			d := document.NewFromJSON([]byte(test.data))
+
+			fb := document.NewFieldBuffer()
+			err := fb.Copy(d)
+
+			if test.fails {
+				require.Error(t, err)
+			} else {
+				require.NoError(t, err)
+				require.Equal(t, *test.expected, *fb)
+			}
+		})
+	}
+
+	t.Run("GetByField", func(t *testing.T) {
+		d := document.NewFromJSON([]byte(`{"a": 1000}`))
+
+		v, err := d.GetByField("a")
+		require.NoError(t, err)
+		require.Equal(t, document.NewIntegerValue(1000), v)
+
+		v, err = d.GetByField("b")
+		require.Equal(t, document.ErrFieldNotFound, err)
+	})
+}
+
+func BenchmarkJSONToDocument(b *testing.B) {
+	data := []byte(`{"_id":"5f8aefb8e443c6c13afdb305","index":0,"guid":"42c2719e-3371-4b2f-b855-d302a8b7eab0","isActive":true,"balance":"$1,064.79","picture":"http://placehold.it/32x32","age":40,"eyeColor":"blue","name":"Adele Webb","gender":"female","company":"EXTRAGEN","email":"adelewebb@extragen.com","phone":"+1 (964) 409-2397","address":"970 Charles Place, Watrous, Texas, 2522","about":"Amet non do ullamco duis velit sunt esse et cillum nisi mollit ea magna. Tempor ut occaecat proident laborum velit nisi et excepteur exercitation non est labore. Laboris pariatur enim proident et. Qui minim enim et incididunt incididunt adipisicing tempor. Occaecat adipisicing sint ex ut exercitation exercitation voluptate. Laboris adipisicing ut cillum eu cillum est sunt amet Lorem quis pariatur.\r\n","registered":"2016-05-25T10:36:44 -04:00","latitude":64.57112,"longitude":176.136138,"tags":["velit","minim","eiusmod","est","eu","voluptate","deserunt"],"friends":[{"id":0,"name":"Mathis Robertson"},{"id":1,"name":"Cecilia Donaldson"},{"id":2,"name":"Joann Goodwin"}],"greeting":"Hello, Adele Webb! You have 2 unread messages.","favoriteFruit":"apple"}`)
+
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		d := document.NewFromJSON(data)
+		d.Iterate(func(string, document.Value) error {
+			return nil
+		})
+	}
+}
diff --git a/document/document_test.go b/document/document_test.go
index ec5784cd..768fe412 100644
--- a/document/document_test.go
+++ b/document/document_test.go
@@ -83,7 +83,6 @@ func TestFieldBuffer(t *testing.T) {
 	})
 
 	t.Run("Set", func(t *testing.T) {
-
 		tests := []struct {
 			name  string
 			data  string
@@ -118,9 +117,8 @@ func TestFieldBuffer(t *testing.T) {
 			t.Run(tt.name, func(t *testing.T) {
 				var fb document.FieldBuffer
 
-				d, err := document.NewFromJSON([]byte(tt.data))
-				require.NoError(t, err)
-				err = fb.Copy(d)
+				d := document.NewFromJSON([]byte(tt.data))
+				err := fb.Copy(d)
 				require.NoError(t, err)
 				p, err := parser.ParsePath(tt.path)
 				require.NoError(t, err)
diff --git a/sql/query/expr/expr_test.go b/sql/query/expr/expr_test.go
index cbd4da65..bf12df81 100644
--- a/sql/query/expr/expr_test.go
+++ b/sql/query/expr/expr_test.go
@@ -13,13 +13,11 @@ import (
 )
 
 var doc document.Document = func() document.Document {
-	d, _ := document.NewFromJSON([]byte(`{
+	return document.NewFromJSON([]byte(`{
 		"a": 1,
 		"b": {"foo bar": [1, 2]},
 		"c": [1, {"foo": "bar"}, [1, 2]]
 	}`))
-
-	return d
 }()
 
 var stackWithDoc = expr.EvalStack{
diff --git a/sql/query/expr/path_test.go b/sql/query/expr/path_test.go
index a52dfd4b..c8f3a267 100644
--- a/sql/query/expr/path_test.go
+++ b/sql/query/expr/path_test.go
@@ -1,6 +1,7 @@
 package expr_test
 
 import (
+	"encoding/json"
 	"testing"
 
 	"github.com/genjidb/genji/document"
@@ -16,8 +17,10 @@ func TestPathExpr(t *testing.T) {
 	}{
 		{"a", document.NewIntegerValue(1), false},
 		{"b", func() document.Value {
-			d, _ := document.NewFromJSON([]byte(`{"foo bar": [1, 2]}`))
-			return document.NewDocumentValue(d)
+			fb := document.NewFieldBuffer()
+			err := json.Unmarshal([]byte(`{"foo bar": [1, 2]}`), fb)
+			require.NoError(t, err)
+			return document.NewDocumentValue(fb)
 		}(),
 			false},
 		{"b.`foo bar`[0]", document.NewIntegerValue(1), false},
@@ -30,12 +33,11 @@ func TestPathExpr(t *testing.T) {
 		{"d", nullLitteral, false},
 	}
 
-	d, err := document.NewFromJSON([]byte(`{
+	d := document.NewFromJSON([]byte(`{
 		"a": 1,
 		"b": {"foo bar": [1, 2]},
 		"c": [1, {"foo": "bar"}, [1, 2]]
 	}`))
-	require.NoError(t, err)
 
 	for _, test := range tests {
 		t.Run(test.expr, func(t *testing.T) {