Skip to content

Conversation

@pcmoritz
Copy link
Contributor

No description provided.

Copy link
Member

Choose a reason for hiding this comment

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

Please don't do sudo calls in unit tests. This could rather be a script that is run as an extra step in the CI. Still, this does not seem to test any Arrow/Plasma functionality?

Copy link
Contributor

Choose a reason for hiding this comment

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

The filesystem created here is used to test Plasma support for huge pages, which is a performance optimization.

@pcmoritz pcmoritz changed the title ARROW-2215: [Plasma] hugetables munmap issue ARROW-2215: [Plasma] Hugetables munmap issue Feb 27, 2018
@pcmoritz
Copy link
Contributor Author

pcmoritz commented Feb 27, 2018

Thanks for the tip about sudo. The test was not complete yet. It is now ready, the test failure is unrelated.

@robertnishihara
Copy link
Contributor

Looks good to me. Consider adding a comment in malloc.cc that if we change

pointer = pointer_advance(pointer, sizeof(size_t));
then we need to modify the client as well.

@robertnishihara
Copy link
Contributor

I believe @xhochy's comment is addressed, so I will attempt to merge this.

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.

3 participants