On Monday, 20 May 2013 at 1:35 PM, SASADA Koichi wrote:
> 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?
> 
> 

funny_falcon explained this well. It's significantly faster in this case when compared to st_table.
 
> 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.
I think this is a reasonable proposal. I'll add it. 
 
> # minor comment: `sa_' prefix is too short :P
What would you suggest? Ruby already exports symbols with short prefixes, eg. st_. 
 
> # minor comment: change of ext/extmk.rb seems not needed
> https://github.com/charliesome/ruby/compare/trunk...klasscache-trunk#L4L219
> 
> 

Whoops, fixed! Thanks for pointing this out. 
 
> # 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
> 
> 

This is also a reasonable suggestion. I have introduced a new vm_state_version_t typedef.

Thanks for your feedback!