Improve Rubinius support for msgpack Ruby gem #103
Merged
Conversation
This file contains 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
Using internals of MRI by using RARRAY_PTR makes it necessary for other implementations such as Rubinius to continuously copy the structure returned by RARRAY_PTR back and forth since in Rubinius objects are layed out differently internally. Extensions should not depend and use these internal MRI structures if this is not necessary and when there are API methods that can provide the same functionality. This makes sure other implementations can also use the extension without any big problems. For this reason I also removed the FIXME comment, since that change would also heavily depend on the internal memory layout of objects on MRI.
From what I could investigate, msgpack doesn't modify char* buffers obtained from RSTRING_PTR. This means that on Rubinius we don't have to copy back and forth the buffer to make sure it's also updated on the Ruby side. This copying of buffers is a similar problem as the RARRAY_PTR problem, because it is not safe to expose GC'ed memory on Rubinius to extensions since it can move due to Rubinius having a moving GC.
repeatedly
added a commit
that referenced
this pull request
Mar 19, 2012
Improve Rubinius support for msgpack Ruby gem
|
@dbussink : Thanks for the pull request. It's a great patch! |
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.
These commits improve the support for Rubinius in the msgpack gem. It makes sure the msgpack gem doesn't use internals of MRI to handle some things, because emulating them makes it terribly slow in Rubinius. By using the functions available in MRI, this is not a problem.
It also improves how the st.h header is detected by not using the RUBY_VM define, but by letting extconf.rb detect whether it's present or not.