Skip to content

Conversation

@jf-tech
Copy link
Owner

@jf-tech jf-tech commented Oct 9, 2020

As a result, adding Release(*idr.Node) to FormatReader so that Ingester will free up *idr.Node allocations for recycling

Updated all benchmarks.

Not much degradation so far. The reason the json sample bench mark didn't improve much is because there isn't much nodes allocated per read. However typically in the CSV parsing/transform scenarios, files are usually very long and node allocation caching saving would start to show significantly.

resolves #83

jf-tech added 6 commits October 9, 2020 18:10
The key diff/improvement is now we add xpath to the csv reader so that we can perform both
positive row selection as well as negative row skipping. Much more powerful than the old
row skipping thingy.

Next PR will bring the FileFormat implementation in, which will complete the csv fileformat.
As a result, adding `Release(*idr.Node)` to `FormatReader` so that `Ingester` will free up `*idr.Node` allocations for recycling

Updated all benchmarks.

No degradation so far. The reason the json sample bench mark didn't improve much is because there isn't
much nodes allocated per read. However typically in the CSV parsing/transform scenarios, files are usually
very long and node allocation caching saving would start to show significantly.
@jf-tech jf-tech linked an issue Oct 9, 2020 that may be closed by this pull request
@jf-tech
Copy link
Owner Author

jf-tech commented Oct 9, 2020

all right i gave up. this PR it is. no checks that's fine. already ran tests many times locally.

@codecov
Copy link

codecov bot commented Oct 9, 2020

Codecov Report

Merging #86 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #86   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           35        35           
  Lines         1552      1587   +35     
=========================================
+ Hits          1552      1587   +35     
Impacted Files Coverage Δ
customfuncs/javascript.go 100.00% <100.00%> (ø)
handlers/omni/v2/fileformat/csv/reader.go 100.00% <100.00%> (ø)
handlers/omni/v2/fileformat/json/reader.go 100.00% <100.00%> (ø)
handlers/omni/v2/fileformat/xml/reader.go 100.00% <100.00%> (ø)
handlers/omni/v2/ingester.go 100.00% <100.00%> (ø)
handlers/omni/v2/transform/parse.go 100.00% <100.00%> (ø)
idr/jsonreader.go 100.00% <100.00%> (ø)
idr/node.go 100.00% <100.00%> (ø)
idr/xmlreader.go 100.00% <100.00%> (ø)

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 d44a0be...a6b0b22. Read the comment docs.

Copy link
Collaborator

@liangxibing liangxibing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jf-tech jf-tech merged commit 9c55da7 into master Oct 10, 2020
@jf-tech jf-tech deleted the nodecache2 branch October 10, 2020 18:28
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.

Add idr.Node pool and recycling to save repeated allocation

3 participants