Adding csv2 file format. Fixing a major bug in the csv2.reader implementation
#179
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ementation Because we use `encoding/csv.Reader.ReuseRecord`, each call of `csv.Reader.Read()` return value `[]string` is a cached slice. Given we need to potentially cache multiple lines thus multiple calls to `Read()`, what's in the `linesBuf.record` is always duplicate!! We could fix this problem trivially by turning `ReuseRecord` off, but that would incur an allocation cost for vast majority of single-line csv operation. That is completely undesired. So instead, we ourselves cache all the returned strings from (potentially multiple) calls to `csv.Reader.Rdad()` into `reader.records []string` slice, that managing that buffer ourselves, thus practically eliminate the over-allocation problem. Accordingly, in the `reader.linesBuf`, instead of having a `record []string`, we have `recordStart` and `recordNum` to reference into the `reader.records`.
Codecov Report
@@ Coverage Diff @@
## master #179 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 52 53 +1
Lines 2971 3020 +49
=========================================
+ Hits 2971 3020 +49
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Because we use
encoding/csv.Reader.ReuseRecord, each call ofcsv.Reader.Read()return value[]stringis a cached slice. Given we need to potentially cache multiple lines thus multiple calls toRead(), what's in thelinesBuf.recordis always duplicate!!We could fix this problem trivially by turning
ReuseRecordoff, but that would incur an allocation cost for vast majority of single-line csv operation. That is completely undesired.So instead, we ourselves cache all the returned strings from (potentially multiple) calls to
csv.Reader.Rdad()intoreader.records []stringslice, that managing that buffer ourselves, thus practically eliminate the over-allocation problem. Accordingly, in thereader.linesBuf, instead of having arecord []string, we haverecordStartandrecordNumto reference into thereader.records.