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

[Sys] Increased Read Buffers to Reduce Total Syscalls #2814

Conversation

AlexandreSinger
Copy link
Contributor

This change was originally proposed by @heshpdx

See PR #2772

By increasing the read buffer, the number of syscalls can be reduced since more of the large files are read per call.

Copy link
Contributor

@vaughnbetz vaughnbetz left a comment

Choose a reason for hiding this comment

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

Couldn't we just #define big buffer sizes? That seems less complex and equally useful.

libs/libpugiutil/src/pugixml_loc.cpp Outdated Show resolved Hide resolved
libs/libvtrutil/src/vtr_digest.cpp Outdated Show resolved Hide resolved
This change was originally proposed by @heshpdx

See PR verilog-to-routing#2772

By increasing the read buffer, the number of syscalls can be reduced
since more of the large files are read per call.
@AlexandreSinger
Copy link
Contributor Author

@vaughnbetz Because these are across different libraries, I could not cleanly allow them to share the same DEFINE (without a terrible header file, which I did not like). Instead, they would need a define per library.

I agree with your points above. I have changed the default to 1MB and removed the logic in the CMake from directly changing the values (since we are setting them to 1MB anyways).

Let me know what you think.

@vaughnbetz
Copy link
Contributor

LGTM. Will merge as soon as CI passes.

@vaughnbetz vaughnbetz merged commit c8d3111 into verilog-to-routing:master Nov 18, 2024
37 checks passed
@heshpdx
Copy link
Contributor

heshpdx commented Nov 19, 2024

1 MB will be good for the next 20 years. 😄

@vaughnbetz
Copy link
Contributor

And in 21 years, it will be Alex's problem :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants