| Subject: | BUG w/FIX: USING => ParseRegExp silently fails (100x slower) on 2nd, 3rd, .. compile |
| Date: | Sun, 13 Sep 2009 02:20:37 -0700 |
| To: | bug-Inline [...] rt.cpan.org |
| From: | Chris Pirazzi <chris_pirazzi [...] hotmail.com> |
I have ActivePerl 5.10.0 build Build 1003 on Windoze XP.
I have Inline 0.45, which is the latest as I report this.
I am working on a large Inline::C project and I rely on USING => ParseRegExp
for much faster C parsing.
I found that USING => ParseRegExp gave me a 100x speedup for the first C
chunk I compiled, but that subsequent chunks in the same invocation of Perl
were 100x slower.
That is because there is a bug in Inline.pm push_overrides() which causes
Inline to use the older, 100x slower ParseRecDescent after the first
compile, regardless of USING.
The bug is that in Inline.pm push_overrides():
*{$ilsm . "::$override"} =
*{$using_module . "::$override"};
should be
*{$ilsm . "::$override"} =
\&{$using_module . "::$override"};
This is the code that replaces the function Inline::C::get_parser() with the
one from Inline::C::ParseRegExp::get_parser().
The first (incorrect) statement doesn't just overwrite the CODE slot of
get_parser: it replaces the whole glob.
This is a bug because when we go into pop_overrides() to restore things, the
correct statement:
*{$override} = $o->{OVERRIDDEN}{$override};
ends up changing BOTH Inline::C::get_parser() and
Inline::C::ParseRegExp::get_parser() to the saved coderef, which is that of
the original Inline::C::get_parser().
So the effect is that the ParseRegExp version of get_parser() is lost
forever and replaced with the slow ParseRecDescent one.
Man, did this take a long time to figure out.
The implementation of USING is hugely over-complicated to Rube Goldberg
proportions, using symbolic refs and symbol table modification where none is
necessary. It would be better to simply take the USING string and have
Inline::C instantiate an object with it, as in $parser =
$using_string->new(). Or if you're addicted to symbolic refs, then just
have the call to get_parser() in Inline::C parse() do one symbolic ref with
the string, as in
{
no strict 'refs';
my $get_parser_func = \&{ $using_string . "::get_parser" };
}
my $parser = $get_parser_func->($o);
That way, the symbol table never has to get hacked. All that *\&{} makes
the code difficult to maintain and debug.
Barring a cleanup, we should at least change the * to \& so it works.
- Chris Pirazzi