Skip to content
This repository was archived by the owner on May 10, 2024. It is now read-only.

Conversation

@wesm
Copy link
Member

@wesm wesm commented Feb 3, 2016

I twiddled this a bit to cut the runtime in half. I'd like to reduce it further but looking for feedback -- my preference would be to use system entropy (std::random_device) to seed the PRNG and print the seed on failure. So we could run far fewer tests (e.g. only 50 or 100 or so) and occasionally run into flakiness or failure if we refactor and break something internally. Thoughts?

@julienledem
Copy link
Member

if we randomly generate input data we can also print the corresponding input on failure.
That way it is easy to add it as a fixed test case for the input that made it fail.

@wesm
Copy link
Member Author

wesm commented Feb 3, 2016

I'll see if I can make each iteration a function of a single randomly generated seed, and print that on failure, so in the event of a random failure it will be reproducible. Then we can trim the runtime down to a few hundred ms or less

Copy link
Contributor

Choose a reason for hiding this comment

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

Since iters < niters = 500, this condition is always false.

Copy link
Member Author

Choose a reason for hiding this comment

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

Note: I didn't write this code. Will clean it up more per the comments (and reducing runtime further)

Copy link
Contributor

Choose a reason for hiding this comment

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

This is only a performance-improving suggestion. ;)

@wesm
Copy link
Member Author

wesm commented Feb 5, 2016

@asandryh @julienledem I further shortened the runtime (whole test suite takes < 500ms for me locally) and using device entropy to generate PRNG seeds. On failure the seed is printed. Feel free to merge when build green


// prng setup
std::random_device rd;
std::uniform_int_distribution<int> dist(1, std::numeric_limits<int>::max());
Copy link
Contributor

Choose a reason for hiding this comment

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

A small optimization: std::uniform_int_distribution<int> dist(1,20);
and change line 366 to int group_size = dist(gen);

@asandryh
Copy link
Contributor

asandryh commented Feb 6, 2016

lgtm

@wesm
Copy link
Member Author

wesm commented Feb 6, 2016

Done, thanks

@julienledem
Copy link
Member

+1

@asfgit asfgit closed this in a5892f5 Feb 6, 2016
@wesm
Copy link
Member Author

wesm commented Feb 6, 2016

thank you!

@wesm wesm deleted the PARQUET-507 branch February 6, 2016 20:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants