Named based constant cache invalidation

[haileys]

Hi!

I've implemented named based constant cache invalidation in JRuby.

In my implementation, I've added a hashtable mapping constant names to Invalidator objects on the Ruby class. The lookup of the Invalidator object is cached in fields of certain classes (example) - I've tried to make this threadsafe, but I'd like my approach verified because I've probably made a mistake somewhere.

I used this script to benchmark the performance improvement of this patch:

require "benchmark"

class A
  Foo = 123

  class B
    class C
      class D
        class E
          def self.get_foo
            Foo
          end
        end
      end
    end
  end
end

Bar = 456

5.times do
  puts Benchmark.measure {
    1_000_000.times do
      Object.send(:remove_const, :Bar)
      Object.const_set(:Bar, 456)
      A::B::C::D::E.get_foo
    end
  }
end

ruby-2.0.0-p0:

  0.930000   0.000000   0.930000 (  0.935408)
  0.940000   0.000000   0.940000 (  0.937916)
  0.970000   0.000000   0.970000 (  0.967160)
  0.940000   0.000000   0.940000 (  0.937358)
  0.930000   0.000000   0.930000 (  0.931662)

JRuby master:

  1.250000   0.010000   1.260000 (  0.865000)
  0.700000   0.000000   0.700000 (  0.670000)
  0.700000   0.010000   0.710000 (  0.686000)
  0.670000   0.000000   0.670000 (  0.677000)
  0.680000   0.000000   0.680000 (  0.679000)

JRuby constant-name-cache:

  1.250000   0.010000   1.260000 (  0.794000)
  0.620000   0.000000   0.620000 (  0.589000)
  0.580000   0.010000   0.590000 (  0.572000)
  0.570000   0.000000   0.570000 (  0.568000)
  0.570000   0.010000   0.580000 (  0.570000)

✨

[headius]

Mostly good suggestions from @wmeissner. Summing up some points:

1. ConcurrentHashMap would be good. It will be mostly read, so we can set it to concurrency level of 1 and it won't be too big then.
2. When a new invalidator is created, appropriate atomic operations must be performed to guarantee exactly one is ever created. If two get created, you could see threads stomp on each other and someone cache an invalidator that will never be invalidated. The synchronized modifier on getConstantValidator will do this, but we don't want to synchronize on every read either. CHM.putIfAbsent should do the trick. (Note: This may apply to your Rubinius patch as well).
3. I don't think the invalidator fields need to be volatile; worst case is they get set by multiple threads, but as long as (2) holds, they'll just end up with the same invalidator anyway. And CHM will guarantee volatility of the invalidator state (happens-before).

We can attempt to make these changes for you, or if you like you can try them yourself. They're not major, and the patch looks good otherwise.

[haileys]

@wmeissner @headius Thanks for the fantastic feedback!

I've gone ahead and implemented your suggestions, also I've removed the synchronization from the invalidator accessor methods on the AST and IR classes because I figure if multiple threads race on this, the worst that can happen is invalidator is set twice. Let me know what you think.

[headius]

Looks good! I'll get this merged and play with it a bit.

One note about the benchmark: We don't handle caching Colon2 as well as we could, so there could be considerably more improvement if we did that. Your improvement here could make the entire :: chain optimize away to nothing and never invalidate...so that's motivation for me to make another attempt.

[headius]

Merged in 79dad3e. Thanks! Will have to try improving the perf of Colon2 logic in the compiler now.