Use postponed jobs if YJIT is enabled. #180
Conversation
casperisfine force-pushed the yjit-head-compat branch from 002469c to 6155895 Compare 
Fix: tmm1#179 YJIT doesn't support being interrupted by signal in random places, and probably will never support it.
casperisfine force-pushed the yjit-head-compat branch from 6155895 to 179c5ac Compare 
| I got some weird failures on 2.3 and 2.4, but not sure it's worth digging into it, I'd be tempted to make 2.5 the minimum tested version. |
| I tested this on our YJIT CI and it does get rid of the VM crashes. |
| * See https://github.com/ruby/ruby/commit/0e276dc458f94d9d79a0f7c7669bde84abe80f21 | ||
| */ | ||
| #if RUBY_API_VERSION_MAJOR < 3 | ||
| stackprof_use_postponed_job = 0; |
There was a problem hiding this comment.
This seems a bug.
If Ruby < 3.0 (or YJIT), we want to use postponed job.
In other words rb_profile_frames() can only be used anywhere for Ruby 3.0+ no YJIT.
This is most likely the cause of https://bugs.ruby-lang.org/issues/18967 and recent CI failures #182 (comment)
| One surprising side effect of this is wildly different profiles when using YJIT vs not, and some of these differences are not because of JITed code but because of postponed jobs when using YJIT vs not. For instance I'm looking at YARP parse, dump and load performance (on branch varint). 3.2.1 with YJIT:
For instance 19.2% |
| Note that historically, stackprof would just always go through postponed jobs. It's only since #150 that postponed jobs are skipped if you are on Ruby 3+. So yeah, enabling YJIT introduce some amount of inaccuracy, but I don't see what we can do about it :/ |
| In the JVM, there's a ~unofficial API that profilers such as async-profiler where they can still sample from the signal handler, even in the presence of JIT and other weird VM states (such as during GC). That's obviously a non-trivial approach, but making |
| When I discussed this with the YJIT team, they said it was unlikely this would ever be supported. I only have limited knowledge here, but I get that it's tricky to do. |
| I wonder if it's mostly a performance concern (e.g. limiting the kind of optimizations that can be done, or being more costly having to push the stack frames), or if there's other concerns as well. E.g. if the main concern was performance, it occurs to me that it would be interesting to maybe make it an option to "go slower, but more observable". That way we could enable the flag we you really need more visibility, with knowledge that Ruby would go slower. |
| IIRC it was both, as it both complicate the code and made it slower. That conversation was a long time ago. But overall I agree that Ruby profiling APIs are less than ideal, and some work upstream would be nice. |
| I think the main question is why is it unsafe to use |
The conversation was too long ago, so I'll defer to @tenderlove here. |
| @eregon At the time we did this, YJIT didn't "atomically" write Ruby frames. It would push a frame before filling out the frame's memory, and if The redmine ticket you're linking to is with regard to C stack frames, Regardless, I don't think this PR has anything to do with the redmine issue. |
Is there any performance advantage to do so? I'm thinking "push a frame" is basically updating some pointer by writing to memory, so performance-wise it might be identical to do it in a different order. Doing it like the interpreter ("push" last) would then let rb_profile_frames() work as-is probably. And in fact the JIT can easily choose the order of writes since it emits the assembly, unlike the interpreter. But maybe it's about filling the frame lazily and only when needed, e.g., just before a side exit?
Ah, my bad, thanks for clarifying. |
| Thanks for the information here, y’all. My question is: can we currently trust |
7 participants
Fix: #179
YJIT doesn't support being interrupted by signal in random places, and probably will never support it.
Basically turn off #150, if YJIT is enabled.
cc @tenderlove @XrXr @jhawthorn