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

Slightly more efficient conversion from JSON to document #270

Merged
merged 5 commits into from
Oct 23, 2020

Conversation

asdine
Copy link
Collaborator

@asdine asdine commented Oct 17, 2020

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
Copy link

codecov-io commented Oct 17, 2020

Codecov Report

Merging #270 into master will increase coverage by 0.69%.
The diff coverage is 81.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #270      +/-   ##
==========================================
+ Coverage   61.35%   62.04%   +0.69%     
==========================================
  Files          68       68              
  Lines        6345     6450     +105     
==========================================
+ Hits         3893     4002     +109     
+ Misses       1936     1930       -6     
- Partials      516      518       +2     
Impacted Files Coverage Δ
document/create.go 68.27% <72.72%> (+0.15%) ⬆️
database/table.go 57.25% <100.00%> (ø)
document/array.go 73.33% <100.00%> (+0.45%) ⬆️
document/document.go 75.93% <100.00%> (+1.06%) ⬆️
engine/memoryengine/store.go 91.96% <0.00%> (-1.79%) ⬇️
sql/parser/expr.go 84.10% <0.00%> (+6.29%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8fc7000...9b97fca. Read the comment docs.

document/create.go Outdated Show resolved Hide resolved
@tie
Copy link
Contributor

tie commented Oct 18, 2020

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

@asdine
Copy link
Collaborator Author

asdine commented Oct 23, 2020

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)

@asdine asdine merged commit 08dddc3 into master Oct 23, 2020
@asdine asdine deleted the lazy-json-to-document branch October 23, 2020 06:49
@asdine asdine added this to the v0.9.0 milestone Nov 11, 2020
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