-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Let the GC update call data information #2698
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
8abf8ff
to
3839127
Compare
I can't understand the idea. Could you write some pseudo algorithm code? |
@ko1 I've added debug code to ensure the compactor updates all inline caches, including static caches. Algorithm is like this: $compact_count = 0
def update_references
heap.each_object do |object|
case object
when RubyVM::InstructionSequences
instruction_sequences.each do |iseq|
iseq.operands.each do |operand|
case operand
when InlineCache
inline_cache = operand
inline_cache.compact_count = $compact_count
inline_cache.me = rb_gc_location(inline_cache.me)
end
end
end
else
# other updates
end
end
def gc_compact
$compact_count += 1
move_objects
update_references
end
def vm_sendish cfp, inline_cache
if inline_cache.compact_count != $compact_count
raise "GC did not update references for this inline cache"
end
end
def execute_iseq
iseq.instructions.each do |instruction, operands|
case instruction
when :opt_send_without_block
inline_cache = operands
vm_sendish cfp, inline_cache
else
# other instructions
end
end
end
def mutator
loop do
execute_iseq
gc_compact
end
end I need to add |
Thank you. def vm_sendish cfp, inline_cache
if inline_cache.compact_count != $compact_count
raise "GC did not update references for this inline cache"
end
end my concern is how normal method dispatchs are affected. the above pseudo-code raises an exception. is it intentional? or only assertion? I can't accept such exception for normal method dispatch. |
This is only an assertion. I want to make a special CI job to test that all inline caches are updated correctly. Final patch will be more like:
|
5a91cea
to
3b2c776
Compare
3b2c776
to
c2799f4
Compare
I'm rewriting inline method call algorithm completely and it will support relocation. |
@ko1 should I close this PR? I want to finish automatic compaction, and I need relocation support. 😄 |
7e44f04
to
a6fae06
Compare
Currently, compaction will invalidate all inline caches. I would like to update the compactor to fix references in inline caches. Unfortunately this static variable is not reachable from the GC. This commit introduces a "call data" imemo type to wrap the call data. This way the GC can see the static call data variable and update its references.
Make sure the compact count on call cache is initialized correctly so that we can confirm inline cache values are updated Co-Authored-By: John Hawthorn <john@hawthorn.email>
Since the caches are essentially weak references, the method entry could get collected. In that case we'll get a T_NONE back or some unrelated object back from the GC. In that case we just want to ignore it.
This benchmark is to ensure performance of `rb_funcallv`.
b6988ec
to
7b19e8c
Compare
Now that GC can update inline caches, we can stop invalidating them on compaction.
7b19e8c
to
13bd332
Compare
Call cache performance:I expect this patch to have no impact to most call caches and very small impact to some call caches. There are two types of call caches, one defined in Ruby and the other defined as static variables. Ruby Call CachesThese caches are allocated during script compile time and are store in the instruction sequences. This patch does not change the runtime behavior of call caches defined in Ruby. For example: class Foo
def bar(x)
x.foo # This runtime behavior doesn't change
end
def foo; end
end
x = Foo.new
x.bar x This patch changes the compaction behavior. It updates references so that we get cache hits even though compaction occurred. Static Call CachesThis patch does change the behavior of static call caches. Static call caches are typically defined in the GC cannot see static variables, so this patch introduces a wrapper object. The GC can see the wrapper object, so it can update references stored in the cache. Code ImpactOnly I think that branch prediction will make this negligible compared to the original. Performance Tests
Benchmark results:
The version of Inline Cache ImprovementsTo measure inline cache improvements, I recompiled Ruby to output cache hit / miss debug counters. Here is the test program: class CachedCalls
eval ('aa'..'zz').map { |x| "def m#{x}(x); x.foo; end" }.join("\n")
eval "def call(x);" + ('aa'..'zz').map { |x| "m#{x}(x);" }.join("\n") + "; end"
end
class Foo; def foo; end end
cc = CachedCalls.new
foo = Foo.new
100.times do
cc.call foo
GC.compact
end This program will demonstrate inline cache hit and misses when GC compaction is executed. Here are the counters from
Here are the counters from this branch:
This patch ensures inline caches are updated so that we get hits instead of misses. Static Inline Cache ImprovementsWe can demonstrate that static inline caches are updated during GC compaction as well: class CachedCalls
def funcallv
list = ('a'..'z').to_a
1_000.times do
list.grep('z') { 'z' }
GC.compact
end
end
end
class Foo; def foo; end end
cc = CachedCalls.new
cc.funcallv This benchmark calls
This branch:
We can see the inline cache hits increased with this branch. |
Thank you for measurements. For example, the following script generates a script running N classes and invoke methods for them. For smaller N, all of calldata are in CPU cache. For bigger N, it emulates bigger apps. N = ARGV.shift.to_i
L = 100_000_000 / N
puts "L=#{L}"
puts N.times.map{|i| "class C#{i}; def foo; end; end"}.join("\n") + "\n" +
N.times.map{|i| "o#{i} = C#{i}.new" }.join("\n") + "\n"
puts 'L.times{'
puts N.times.to_a.shuffle.map{|i| "o#{i}.foo"}.join("\n")
puts '}' Anyway, as I wrote that I'm rewriting inline method cache algorithm and I can't accept this approach. My patch is similar, but introduce CC sharing with new CC invalidation protocol. |
@ko1 no problem! Thanks for letting me know what you were looking for. Next time I need to benchmark CCs I'll use this info! |
These patches let the GC update call data information, including call data used on static inline caches.
I've added a
compact_count
to the inline cache so that we can verify if all caches were updated. When the cache is read, we compare the compact count stored on the cache to the compact count in the GC. If those counts match, then we know the compactor was able to update references in the cache.This patch isn't 100% ready yet, we need to make the
compact_count
commented out normally so that there is more room for inline cache entries. This is just to see if CI passes.