Skip to content
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

Consider re-evaluating use of constants for hash lookups. #9

Open
kentfredric opened this issue May 20, 2015 · 0 comments
Open

Consider re-evaluating use of constants for hash lookups. #9

kentfredric opened this issue May 20, 2015 · 0 comments

Comments

@kentfredric
Copy link

As per IRC discussion, this should be re-visited at some point in the future.

Its very likely that this approach causes more harm good for the codebase, and as well intentioned as the technique is, there are more ways to mess it up than there is with plain constants.

There's an incredibly marginal benefit on perls before 5.8.9 ( like, 1% performance difference of const vs string lookup on pure hash lookups ), but I haven't got any perls since then that exhibit that difference, so its not worth retaining it for that alone.

IRC log for context follows:

07:38:43 <@kentnl> Exodist: not a challenge, just a question: Which demographics in performance indicated the need for the $hash->{+CONST} trick?
07:39:03 <@kentnl> I've done my own benchmarks and I have an idea why, I just want to know how you reached that conclusion. =)
07:39:14 <@kentnl> ( mostly a just "For the record in case somebody asks" )
07:58:00 <@Exodist> kentnl, ok, we can document it.
07:58:04 <@Exodist> but for you info:
07:58:58 <@Exodist> Early Test-Stream iterations were super slow, using accessors is important for encapsulation, but using the get accessors for internal sas well was adding a lot of time, and profiling showed it as a significant chunk.
07:59:28 <@Exodist> Changing it so that objects use the constant internally instead fo get-accessors (or even worse, get/set accessors) made it significantly more performant.
08:00:01 <@Exodist> We also tried using array based objects where the constants where array indexes, but ultimately found that the performance increase over hashes was not notable enough to be worth the complication and lack of extendability it brought
08:14:49  * kentnl nods
08:15:10 <@kentnl> was the performance difference of $hash->{'const'} vs $hash->{+CONST} significant on all perls? Or just specific versions
08:15:46 <@kentnl> I've replicated a performance difference of about 1% in a microbenchmark on 5.8.9, but I can't seem to replicate a performance difference of significance less than system noise on any other perl.
08:16:10 <@Exodist> oh, no ther eis no perf difference between constant and string that I noticed, the use of constants was to catch typos, wheras if you do $foo->{'typo'} it just chugs along no problem,  but $foo->{+TYPO} will fail since the typo is not a defined sub
08:16:45 <@kentnl> *nods*
08:17:20 <@kentnl> Well, on 5.8.9, +CONST is faster than 'const' by about 0.005 seconds per 8-million lookups. :p 
08:17:23 <@kentnl> its tiny.
08:17:25 <@kentnl> But its there.
08:20:16 <@Exodist> nice
08:22:34 <@kentnl> 08:19:18 < mst> I've worked on code that used that convention for that guarding purpose
08:22:34 <@kentnl> 08:19:22 < mst> it did more harm than good
08:23:03 <@kentnl> I'm not saying "change it". But I tend to respect such an experience.
08:23:05 <@Exodist> I need details on why
08:23:16 <@Exodist> what harm was caused?
08:23:22 <@kentnl> $hash->{CONST} # is just as likely to happen.
08:23:43 <@Exodist> ah, that is a good point
08:24:04 <@kentnl> and also if you're reading code thats broken, you're not going to notice CONST is an error.
08:24:06 <@Exodist> I have also found myself accidentally doing {+lowercase} which also runs, but obviosuly is wrong
08:24:56 <@kentnl> whereas if sombody types 'conts'  for example, anyone with good spelling recognition skills can spot that a mile off.
08:25:04 <@Exodist> for now I want to keep it, but if I notice it causing problems it can be addressed later
08:25:15  * Exodist is very bad at spelling, and makes a lot of typos
08:25:36 <@kentnl> would you open a bug to tag that for later?
08:25:38 <@Exodist> I am far more likely to make a typo in a word name than I am to forget the '+', I suspect the benefit vs downside is very subjective to the person
08:26:14 <@Exodist> you can open the bug, a bvug to re-evaluate it before stable release to see if we want to stick with it or drop it
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant