Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update plotter_disk.hpp #202

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

xorinox
Copy link
Contributor

@xorinox xorinox commented Mar 31, 2021

Setting the rlimit also on Linux was necessary to make it work with 1024 buckets plus changing the number from 600 to 4096.

Setting the rlimit also on Linux was necessary to make it work with 1024 buckets plus changing the number from 600 to 4096.
Copy link
Contributor

@arvidn arvidn left a comment

Choose a reason for hiding this comment

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

looks good apart from the windows build issue.

@@ -73,12 +73,11 @@ class DiskPlotter {
bool show_progress = false)
{
// Increases the open file limit, we will open a lot of files.
#ifndef _WIN32
Copy link
Contributor

Choose a reason for hiding this comment

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

you need to put these #ifndef back. Note the n in there, this condition is to omit this code on windows, because windows doesn't have rlimits. (this is why this patch fails to build on windows, here)

@rostislav
Copy link
Contributor

I would also suggest to make the commit message more descriptive, e.g. "Increase file descriptor limit"

@github-actions
Copy link

'This PR has been flagged as stale due to no activity for over 60
days. It will not be automatically closed, but it has been given
a stale-pr label and should be manually reviewed.'

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.

None yet

3 participants