From: Charlie Somerville <charlie@...>
Date: 2013-05-20T18:21:45+09:00
Subject: [ruby-core:55083] Re: [ruby-trunk - Feature #8426][Open] Implement class hierarchy method caching

--5199eb22_614fd4a1_168
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

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!
--5199eb22_614fd4a1_168
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline


                <div><span style=3D=22color: rgb(160, 160, 168); =22>On M=
onday, 20 May 2013 at 1:35 PM, SASADA Koichi wrote:</span></div>
                <blockquote type=3D=22cite=22 style=3D=22border-left-styl=
e:solid;border-width:1px;margin-left:0px;padding-left:10px;=22>
                    Could you explain the data stracture=3F Patch seems t=
o introduce new data<span><div><div><div>structure =60sparse array'. What=
 is this and how to use it on this patch=3F</div></div></div></span></blo=
ckquote><div>funny=5Ffalcon explained this well. It's significantly faste=
r in this case when compared to st=5Ftable.</div><div>&nbsp;</div><blockq=
uote type=3D=22cite=22 style=3D=22border-left-style:solid;border-width:1p=
x;margin-left:0px;padding-left:10px;=22>And another consern is verificati=
on mechanism of the result. Complex<span><div><div><div>methoc caching me=
chanism introduces bugs because:</div><div>- Everyone make bugs.</div><di=
v>- If someone who doesn't care method cache mechanism adds new</div><div=
>  core feature such as refinement and so on, it will break assumption</d=
iv><div>  about method caching.</div><div>And this bug is difficult to fi=
nd out because they may be rare.</div><div><br></div><div>My proposal is =
to add verify mode (on/off by macro, of course off as</div><div>default) =
which check the cached result using a naive method search.</div><div><br>=
</div><div>  =23define verify 0</div><div>  result =3D ...</div><div>  =23=
if verify</div><div>    if (naive=5Fmethod=5Fsearch() =21=3D result) rb=5F=
bug(...);</div><div>  =23endif</div><div><br></div><div>It will help debu=
gging.</div></div></div></span></blockquote><div>I think this is a reason=
able proposal. I'll add it.&nbsp;</div><div>&nbsp;</div><blockquote type=3D=
=22cite=22 style=3D=22border-left-style:solid;border-width:1px;margin-lef=
t:0px;padding-left:10px;=22>=23 minor comment: =60sa=5F' prefix is too sh=
ort :P</blockquote><div>What would you suggest=3F Ruby already exports sy=
mbols with short prefixes, eg. st=5F.&nbsp;</div><div>&nbsp;</div><blockq=
uote type=3D=22cite=22 style=3D=22border-left-style:solid;border-width:1p=
x;margin-left:0px;padding-left:10px;=22><span><div><div><div>=23 minor co=
mment: change of ext/extmk.rb seems not needed</div><div><a href=3D=22htt=
ps://github.com/charliesome/ruby/compare/trunk...klasscache-trunk=23L4L21=
9=22>https://github.com/charliesome/ruby/compare/trunk...klasscache-trunk=
=23L4L219</a></div></div></div></span></blockquote><div>Whoops, fixed=21 =
Thanks for pointing this out.&nbsp;</div><div>&nbsp;</div><blockquote typ=
e=3D=22cite=22 style=3D=22border-left-style:solid;border-width:1px;margin=
-left:0px;padding-left:10px;=22><span><div><div><div>=23 minor comment: u=
sing uint64=5Ft directly is not preferable.</div><div>  for example:</div=
><div>  =23if HAVE=5FUINT64=5FT</div><div>    typedef version=5Ft uint64=5F=
t;</div><div>  =23else</div><div>    typedef version=5Ft uint=5Ft;</div><=
div>  =23endif</div></div></div></span></blockquote><div>This is also a r=
easonable suggestion. I have introduced a new vm=5Fstate=5Fversion=5Ft ty=
pedef.</div><div><br></div><div>Thanks for your feedback=21</div>
            
--5199eb22_614fd4a1_168--