Skip to content

ARROW-5270: [C++] reduce json-reader-test's working size#4264

Closed
bkietz wants to merge 1 commit intoapache:masterfrom
bkietz:5270-Reenable-Valgrind-on-Travis-CI
Closed

ARROW-5270: [C++] reduce json-reader-test's working size#4264
bkietz wants to merge 1 commit intoapache:masterfrom
bkietz:5270-Reenable-Valgrind-on-Travis-CI

Conversation

@bkietz
Copy link
Copy Markdown
Member

@bkietz bkietz commented May 6, 2019

Valgrind on Travis is slow due to this test, which doesn't need to be so big. @pitrou


TEST(ReaderTest, MultipleChunksParallel) {
int64_t count = 1 << 20;
int64_t count = 1 << 10;
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.

One possible pattern is to choose a different test size if Valgrind is enabled. We do this in other places:

#ifdef ARROW_VALGRIND
int64_t count = ...;  // some moderate number
#else
int64_t count = ...;  // some bigger number
#endif

Ultimately, it's your choice :-)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The test was originally just 1kB. I changed the size around while looking for the benchmark discrepancy, and ended up checking in 1MB. It really doesn't need to be that large, I think

Copy link
Copy Markdown
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@pitrou pitrou closed this in 0882924 May 7, 2019
@bkietz bkietz deleted the 5270-Reenable-Valgrind-on-Travis-CI branch February 25, 2021 16:10
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.

2 participants