Skip to content

Make fread handle cases when allocnrow is too small - #2355

Merged
mattdowle merged 3 commits into
masterfrom
fread_realloc
Sep 14, 2017
Merged

mattdowle merged 3 commits into
masterfrom
fread_realloc

Conversation

@st-pasha

Copy link
Copy Markdown
Contributor
  • Behavior of allocateDT is modified to handle the case when it is called with a different number of nrows than previously
  • If too many rows is found, break out of the main reading loop into a single-threaded context; then call allocateDT with a new estimated number of columns; and finally restart the main reading loop from exactly the same point where we have left, avoiding the need to rescan the entire file again.
  • Added a test
  • Added verbose output in case such "nrows bump" is encountered; and also verbose output about the parameters of the main scan: nJumps, chunkBytes and lastRowEnd-pos

Closes #2246

…allocated for a DT turned out to be less than needed
@st-pasha st-pasha added the fread label Sep 13, 2017
@st-pasha st-pasha added this to the v1.10.6 milestone Sep 13, 2017
@st-pasha st-pasha self-assigned this Sep 13, 2017
@st-pasha
st-pasha requested a review from mattdowle September 13, 2017 21:21
@codecov-io

codecov-io commented Sep 13, 2017 •

Copy link
Copy Markdown

Codecov Report

Merging #2355 into master will increase coverage by 0.01%.
The diff coverage is 97.72%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2355      +/-   ##
==========================================
+ Coverage   91.14%   91.15%   +0.01%     
==========================================
  Files          61       61              
  Lines       11808    11833      +25     
==========================================
+ Hits        10762    10786      +24     
- Misses       1046     1047       +1
Impacted Files Coverage Δ
src/freadR.c 93.22% <100%> (+0.14%) ⬆️
src/fread.c 95.43% <96.77%> (-0.02%) ⬇️

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 204073d...cb6bd99. Read the comment docs.

Comment thread src/fread.c Outdated
DTPRINT("[11] Read the data\n");
DTPRINT(" jumps=[%d..%d), chunk_size=%llu, total_size=%llu\n", jump0, nJumps, (llu)chunkBytes, (llu)(lastRowEnd-pos));
}
ASSERT(allocnrow <= nrowLimit);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Need the values of allocnrow and nrowLimit included in the error message please if assert fails.

@mattdowle
mattdowle merged commit 2f41b0a into master Sep 14, 2017
@mattdowle
mattdowle deleted the fread_realloc branch September 14, 2017 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants