search

Perl-Critic / PPI

53 stars 44 forks source link

Caching ->isa() #11

Open petdance opened 10 years ago

petdance commented 10 years ago

Has anyone looked into caching ->isa() at the PPI::Element level?

The number of calls to ->isa() in a "make nytprof" run of Perl::Critic is staggering, and I have to think that at least some of those are redundant. Maybe it would be a win if each call to ->isa( 'whatever' ) would cache the result of that lookup.

I can go poking at this, but didn't want to waste my time if this was already considered and rejected.

adamkennedy commented 10 years ago

I haven't looked into it before, although I was under the impression that isa already cache it's checks.

I'd be open to investigations in this area, although it should be noted that one gotcha you are likely to encounter ifs that elements do get released sometimes during the parsing process.

There's also memory potential memory bloat issues. I'm not sure of its a show stopper or not, just letting you know it's likely to be a consideration, and to keep an eye on it.

Adam On Dec 18, 2013 8:17 AM, "Andy Lester" notifications@github.com wrote:

Has anyone looked into caching ->isa() at the PPI::Element level?

The number of calls to ->isa() in a "make nytprof" run of Perl::Critic is staggering, and I have to think that at least some of those are redundant. Maybe it would be a win if each call to ->isa( 'whatever' ) would cache the result of that lookup.

I can go poking at this, but didn't want to waste my time if this was already considered and rejected.

— Reply to this email directly or view it on GitHubhttps://github.com/adamkennedy/PPI/issues/11 .

moregan commented 10 years ago

perl5101delta.pod says:

=head1 Performance Enhancements

=over 4

=item *

A new internal cache means that C<isa()> will often be faster.