Refactor chunking algorithm changes #1676
Conversation
rgmz
force-pushed
the
feat/chunking-tests
branch
5 times, most recently
from
fe0d9b5 to
78f67c2
Compare
detect/directory.go
Outdated
| // Buffer to hold file chunks | ||
| reader = bufio.NewReader(f) |
There was a problem hiding this comment.
Using #1432 as a test case. @ahrav @zricethezav any obvious errors with my use of bufio.Reader? It does feel strange to mix os.File, bufio.NewReader, and bytes.NewBuffer.
Before
12:33PM INF scanned ~104857630 bytes (104.86 MB) in 5.63s
12:33PM WRN leaks found: 1
After
1:05PM INF scanned ~104857630 bytes (104.86 MB) in 1.42s
1:05PM WRN leaks found: 1
There was a problem hiding this comment.
This looks correct to me. I agree—it does feel a bit odd to use all three functions, but if I’m understanding this correctly, they each serve different purposes, so it actually makes sense.
There was a problem hiding this comment.
I'm concerned that f.Read and r.ReadByte may not place nicely. E.g., after the first iteration does calling f.Read also progress the internal reader used by bufio? Perhaps I should replace the f.Read call with reader.Read. (Done)
The number of bytes read (from #1670) appears to be consistent. 🤷
| // https://pkg.go.dev/io#Reader | ||
| if n > 0 { | ||
| // Only check the filetype at the start of file. | ||
| if totalLines == 0 { |
There was a problem hiding this comment.
This doesn't need to be run after the first chunk. This is a rough way to check if it's the first chunk.
| } else if lastChar == '\r' || lastChar == ' ' || lastChar == '\t' { | ||
| // The presence of other whitespace characters (`\r`, ` `, `\t`) shouldn't reset the count. | ||
| // (Intentionally do nothing.) |
There was a problem hiding this comment.
This handles #1665 (comment)
rgmz
force-pushed
the
feat/chunking-tests
branch
6 times, most recently
from
f84fcc7 to
dda1f02
Compare
rgmz
force-pushed
the
feat/chunking-tests
branch
from
dda1f02 to
19bede8
Compare
Description:
This is a follow-up to #1665.
Changes include:
reader.go.bufio.Readerindirectory.goto speed up executionChecklist: