Closed danielpclark closed 6 years ago
Perhaps we could run a profiler on the Rust code to understand why children_compat
is so slow. There is https://github.com/AtheMathmo/cpuprofiler that produces pprof-compatible output.
I use pprof a lot at work, it's easy to analyze and explain the output (especially using the flame graph visualization).
That's an excellent idea. I'd likely need to share the profile here and ask for your insights on it as I feel you'd be far more familiar with these things.
@danielpclark my understanding of what OBJECT_INFECT
is doing is that it's setting a tainted mark on the newly created Pathname object if the previous string is already tainted per Ruby's security model.
I'm trying to find a way we can instantiate new Pathname instances without the overhead we're experiencing in
children_compat
andentries_compat
. Looking at the C source code for it:_This was the method I copied from when implementing
Pathname.new_checked
in Rust. It's currently not used. It's best designed to receive a raw rubyValue
._I'm wondering if the
OBJECT_INFECT
macro simply transforms an existing String object into a Pathname (or any) object. This way the same object structure gets all its methods and identity updated without the added memory cost of creating something new. I could be wrong, that's why I want to look into it.The
RBasic
item is implemented in ruby-sys.So my thinking is if we already had a Ruby String
Value
object to work with we could avoid the overhead by implementing object infection. But this may be an entirely mute point since we're working in Rust Strings and have to create a new RubyValue
object in the first place. I think the cost in performance we're experiencing is that in Ruby land thePathname
creation is the only cost they have, where we have to pay for bothPathname
and theString
it takes as a parameter.Of course I could be totally off the mark since Ruby has to create the String
Value
objects for the methodentries
andchildren
just as we do. For me this is mere speculation and theory. I'd like to look more into it.