Front page | perl.perl5.porters |
Postings from March 2012
Re: 5.15.8: Global destruction phase slow?
Thread Previous
|
Thread Next
From:
Dave Mitchell
Date:
March 13, 2012 07:53
Subject:
Re: 5.15.8: Global destruction phase slow?
Message ID:
20120313145339.GC2889@iabyn.com
On Tue, Mar 13, 2012 at 12:54:09PM +0000, Nicholas Clark wrote:
> On Tue, Mar 06, 2012 at 02:35:32PM +0000, Dave Mitchell wrote:
> > On Tue, Mar 06, 2012 at 12:17:27AM +0000, Dave Mitchell wrote:
>
> > > And this is my code. Oh dear. I guess I'll have to think of something
> > > clever.
> > >
> > > basically it goes quadratic when freeing a large hash whose values are
> > > also hashes.
> >
> > Now fixed with
> >
> > commit 5bec93bead1c10563a402404de095bbdf398790f
> > Author: David Mitchell <davem@iabyn.com>
> > AuthorDate: Tue Mar 6 14:26:27 2012 +0000
> > Commit: David Mitchell <davem@iabyn.com>
> > CommitDate: Tue Mar 6 14:26:27 2012 +0000
> >
> > fix slowdown in nested hash freeing
> >
> > Commit 104d7b69 made sv_clear free hashes iteratively rather than recursivel
> > however, my code didn't record the current hash index when freeing a
> > nested hash, which made the code go quadratic when freeing a large hash
> > with inner hashes, e.g.:
> >
> > my $r; $r->{$_} = { a => 1 } for 1..10_0000;
> >
> > This was noticeable on such things as CPAN.pm being very slow to exit.
> >
> > This commit fixes this by squirrelling away the old hash index in the
> > now-unused SvMAGIC field of the hash being freed.
> >
> > M hv.c
> > M sv.c
> > M sv.h
>
> Unfortunately this introduces a SEGV during global destruction for
> t/op/reset.t. I think this is popping up in some configurations on one of
> George Greer's smokers.
Fixed with the commit below.
Which rather leads me to think that any code that uses SvIS_FREED()
is probably buggy by definition, and just indicates a piece of code where
weak (in the general sense) references aren't handled properly.
It also makes me wonder whether whether SvIS_FREED() should be changed
from
((sv)->sv_flags == SVTYPEMASK)
to
(((sv)->sv_flags == SVTYPEMASK) || !(sv)->sv_refcnt)
i.e. it means is freed or is being freed
????
commit e39a63811f671c92ce10198aa285306911af500e
Author: David Mitchell <davem@iabyn.com>
AuthorDate: Tue Mar 13 14:24:15 2012 +0000
Commit: David Mitchell <davem@iabyn.com>
CommitDate: Tue Mar 13 14:24:15 2012 +0000
stop S_forget_pmop() SEGVing
Commit 5bec93be re-purposed the SvMAGIC field of hashes being freed, on
the grounds that (a) any magic had been freed, (b) the refcnt was zero, so
no-one else could mess with the hash.
Unfortunately in the non-threaded case, PMOPs for m?? regexes have a
non-refcounted link back to their stash. When the stash is freed, any subs
in the stash are freed, which frees the PMOPs, which then see the freed
stash, and assume it still has magic because SvMAGIC is non-null.
The quick fix is to check the SvMAGICAL flags first; a longer term fix
would be to avoid the weak ref (e.g. by always using the threaded variant
of PmopSTASH, which stores the stash's *name* rather than a pointer to the
stash).
Affected files ...
M op.c
Differences ...
diff --git a/op.c b/op.c
index cf3fec0..3bbe4f1 100644
--- a/op.c
+++ b/op.c
@@ -730,7 +730,7 @@ S_forget_pmop(pTHX_ PMOP *const o
PERL_ARGS_ASSERT_FORGET_PMOP;
- if (pmstash && !SvIS_FREED(pmstash)) {
+ if (pmstash && !SvIS_FREED(pmstash) && SvMAGICAL(pmstash)) {
MAGIC * const mg = mg_find((const SV *)pmstash, PERL_MAGIC_symtab);
if (mg) {
PMOP **const array = (PMOP**) mg->mg_ptr;
--
But Pity stayed his hand. "It's a pity I've run out of bullets",
he thought. -- "Bored of the Rings"
Thread Previous
|
Thread Next