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

fread() instead of read() + Add GNUC format validator #37

Merged
merged 3 commits into from
Dec 11, 2023

Conversation

moticless
Copy link
Collaborator

@moticless moticless commented Dec 11, 2023

This commit attempts to reflect also part of the work from previous messy commit:

  1. Add GNUC format validator for RDB_log() and RDB_reportError()
  2. fread() instead of read() to improve performance.

Add GNUC format validator for RDB_log() and RDB_reportError()**

In case of GNUC compiplation add attribute((format(printf, X, Y)))
To verify during compilation correctness of the provided FORMAT
versus variadic arguments that follows.

fread() instead of read() to improve performance**

Following a preliminary benchmark that involved reading from a
file-descriptor (fd) I have found out that assisting openfd()
outperform around ~50% better than using lower level read().
This is because when using bare read(), it makes multiple
system-calls to read a small amount of data whereas fread()
attempts to read a big chunk of data to user-space, even if
requested a small amount. I also tested using bare read() with
a buffer that i managed inside readerFileDesc.c and it reached
Similar results to fread().

Note, the benchmark relied on librdb library that is integrated
into another utility (rl_rdbloader), the adjustment is relatively
self-contained and provides a reliable indication of its
effectiveness.

moticless added 2 commits December 11, 2023 17:10
(A DUMMY COMMIT TO REFLECT PART OF PREVIOUS MESSY COMMIT)
Following a preliminary benchmark that involved reading from a
file-descriptor (fd) I have found out that assisting openfd()
outperform around ~50% better than using lower level read().
This is because when using bare read(), it makes multiple
system-calls to read a small amount of data whereas fread()
attempts to read a big chunk of data to user-space, even if
requested a small amount. I also tested using bare read() with
a buffer that i managed inside readerFileDesc.c and it reached
Similar results to fread().

Note, the benchmark relied on librdb library that is integrated
into another utility (rl_rdbloader), the adjustment is relatively
self-contained and provides a reliable indication of its
effectiveness.
@moticless moticless changed the title Fread imporve fread() instead of read() + Add GNUC format validator Dec 11, 2023
oranagra
oranagra previously approved these changes Dec 11, 2023
src/ext/readerFileDesc.c Outdated Show resolved Hide resolved
@moticless moticless merged commit df07dbc into redis:main Dec 11, 2023
2 checks passed
@moticless moticless deleted the fread-imporve branch December 11, 2023 15:37
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