Repository navigation
release the GVL during copy_memory and write_to_memory - #446
Merged
Merged
Conversation
Both calls evaluate the whole lazy pipeline, but they were attached without blocking: true, so FFI held the GVL for the full computation and every other Ruby thread stalled until they returned.
Member
|
Hi @andrewfader, thank you for finding and fixing this dumbness! |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
vips_image_copy_memoryandvips_image_write_to_memoryare attached withoutblocking: true, but both evaluate the whole lazy pipeline. FFI holds the GVL for the full computation, so every other Ruby thread in the process stops until they return. Onlyvips_cache_operation_buildis attached blocking today.We hit this in a Rails app running on Falcon, where requests are fibers on a few threads. Decoding thumbnails with
copy_memory/write_to_memoryin a background thread still froze every other request on that process. A thread ticking every 10ms saw gaps of 225-283ms during the call. Attaching both functions withblocking: truebrought the gaps down to the normal ~15ms.This change attaches both with
blocking: true, the same wayvips_cache_operation_buildis attached.Tests: two new examples in
spec/image_spec.rbrun a ticker thread whilecopy_memory/write_to_memoryevaluate agaussnoise.gaussblurpipeline, and check that the ticker advanced. Without the change the count is exactly 0 on every run, because the ticker can't get the GVL. With it, both pass, including 8/8 runs with every core busy. The full suite passes (128 examples) and standardrb is clean.