It's an interesting issue and a trivial patch. I'm sure it was done as a safety valve figuring the cycles weren't a big deal. I can't think of any reason to keep checking all these meta dependencies over and over either.
I'm +1 on this. -- Kevin A. McGrail VP Fundraising, Apache Software Foundation Chair Emeritus Apache SpamAssassin Project https://www.linkedin.com/in/kmcgrail - 703.798.0171 On Tue, Jan 8, 2019 at 10:31 AM Olivier Coutu <[email protected]> wrote: > We use a lot of custom rules (2k+) and lint time was growing out of > control, over a minute on some machines. > > It appears like *$alreadydone*, the set of rules that have already been > linted, is reset for every test. This makes lint time quadratic in the > number of tests and does not seem to have any use. Moving it out of the > tests loop reduces my lint time from 21 seconds to 3 seconds with no > visible side effect. > > This patch moves it out of the tests loop > > --- /usr/share/perl5/Mail/SpamAssassin/Conf/Parser.pm 2019-01-07 > 17:54:19.244624440 -0500 > +++ /usr/share/perl5/Mail/SpamAssassin/Conf/Parser.pm 2019-01-07 > 17:55:11.948708724 -0500 > @@ -964,13 +964,13 @@ > my ($self) = @_; > my $conf = $self->{conf}; > $conf->{meta_dependencies} = { }; > + my $alreadydone = { }; > > foreach my $name (keys %{$conf->{tests}}) { > next unless ($conf->{test_types}->{$name} > == $Mail::SpamAssassin::Conf::TYPE_META_TESTS); > > my $deps = [ ]; > - my $alreadydone = { }; > $self->_meta_deps_recurse($conf, $name, $name, $deps, $alreadydone); > $conf->{meta_dependencies}->{$name} = join (' ', @{$deps}); > } > > > The result on ok rulesets and on obviously broken rulesets does not change > from pre-patch, and I can't see any negative side effects of moving it out > of the loop. > > I'm not used to posting on dev lists, so please tell me if more > information is needed or if things should be presented differently. > -Olivier >
