unix: don't copy arguments for sync fs requests #479
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
LGTM |
Member
|
Note to self / you (if you feel like): #469 when doing sync reqs we shouldn't need the loop at all. (we currently add the req to a queue) |
bnoordhuis
force-pushed
the
optimize-sync-fs-reqs
branch
from
3ba0a18 to
4e57137
Compare
Member
Author
|
Added a commit that does just that, PTAL. I can split it up in two PRs if you think it's too big. |
Member
|
LGTM! 👍 |
Member
|
@bnoordhuis I plan to make a patch release tonight or tomorrow night, do you want to land this? |
The parentheses are unnecessary because what they wrap are not macro arguments but function arguments that aren't evaluated by the macro. PR-URL: libuv#479 Reviewed-By: Saúl Ibarra Corretgé <[email protected]>
For synchronous file operation requests (ones without a callback), there is no need to make a copy of the arguments because they don't outlive the scope of the function call. PR-URL: libuv#479 Reviewed-By: Saúl Ibarra Corretgé <[email protected]>
Join the watchdog thread unconditionally on exit. Fixes the following
harmless but noisy memory leak:
576 bytes in 1 blocks are possibly lost in loss record 1 of 2
at 0x4C2A9C7: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
by 0x40121B4: _dl_allocate_tls (in /usr/lib64/ld-2.21.so)
by 0x5AEA045: pthread_create@@GLIBC_2.2.5 (in /usr/lib64/libpthread-2.21.so)
by 0x450D3E: process_wait (runner-unix.c:212)
by 0x4067F1: run_test (runner.c:284)
by 0x405EC3: maybe_run_test (run-tests.c:180)
by 0x4058AD: main (run-tests.c:57)
PR-URL: libuv#479
Reviewed-By: Saúl Ibarra Corretgé <[email protected]>
Synchronous file operations don't need an event loop. Permit NULL as the event loop parameter. Fixes: libuv#469 PR-URL: libuv#479 Reviewed-By: Saúl Ibarra Corretgé <[email protected]>
bnoordhuis
force-pushed
the
optimize-sync-fs-reqs
branch
from
4e57137 to
df62b54
Compare
Member
Author
|
Landed in 64f5c93...df62b54. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For synchronous file operation requests (ones without a callback), there
is no need to make a copy of the arguments because they don't outlive
the scope of the function call.
R=@saghul
CI: https://jenkins-iojs.nodesource.com/view/libuv/job/libuv+any-pr+multi/128/