Great work!

Could you explain the data stracture? Patch seems to introduce new data
structure `sparse array'. What is this and how to use it on this patch?


And another consern is verification mechanism of the result. Complex
methoc caching mechanism introduces bugs because:
- Everyone make bugs.
- If someone who doesn't care method cache mechanism adds new
  core feature such as refinement and so on, it will break assumption
  about method caching.
And this bug is difficult to find out because they may be rare.

My proposal is to add verify mode (on/off by macro, of course off as
default) which check the cached result using a naive method search.

  #define verify 0
  result = ...
  #if verify
    if (naive_method_search() != result) rb_bug(...);
  #endif

It will help debugging.


# minor comment: `sa_' prefix is too short :P
# minor comment: change of ext/extmk.rb seems not needed
https://github.com/charliesome/ruby/compare/trunk...klasscache-trunk#L4L219
# minor comment: using uint64_t directly is not preferable.
  for example:
  #if HAVE_UINT64_T
    typedef version_t uint64_t;
  #else
    typedef version_t uint_t;
  #endif


(2013/05/19 19:44), charliesome (Charlie Somerville) wrote:
> 
> Issue #8426 has been reported by charliesome (Charlie Somerville).
> 
> ----------------------------------------
> Feature #8426: Implement class hierarchy method caching
> https://bugs.ruby-lang.org/issues/8426
> 
> Author: charliesome (Charlie Somerville)
> Status: Open
> Priority: Normal
> Assignee: 
> Category: 
> Target version: 
> 
> 
> =begin
> This patch adds class hierarchy method caching to CRuby. This is the algorithm used by JRuby and Rubinius.
> 
> Currently, Ruby's method caches can only be expired globally. This means libraries that dynamically define methods or extend objects at runtime (eg. OpenStruct) can cause quite a significant performance hit.
> 
> With this patch, each class carries a monotonically increasing sequence number. Whenever an operation which would ordinarily cause a global method cache invalidation is performed, the sequence number on the affected class and all subclasses (classes hold weak references to their subclasses) is incremented, invalidating only method caches for those classes.
> 
> In this patch I've also split the (({getconstant})) VM instruction into two separate instructions - (({getclassconstant})) and (({getcrefconstant})). It's hoped that (({getclassconstant})) can start using class hierarchy caching with not much more effort. This change does affect compatibility in a minor way. Without this patch, (({nil::SomeConstant})) will look up (({SomeConstant})) in the current scope in CRuby (but not JRuby or Rubinius). With this patch, (({nil::SomeConstant})) will raise an exception.
> 
> The patch and all its commits can be viewed here: https://github.com/charliesome/ruby/compare/trunk...klasscache-trunk
> 
> Big thanks to James Golick, who originally wrote this patch for Ruby 1.9.3.
> =end
> 
> 


-- 
// SASADA Koichi at atdot dot net