Bug #97535 for Clone: Potential Speed up by utilizing CoW on >5.20

Sat Jul 26 21:17:59 2014 KENTNL [...] cpan.org - Ticket created

Subject:

Potential Speed up by utilizing CoW on >5.20

Perl >=5.20 now has copy-on-write strings, so on such a perl, if possible, it may be advantageous to utilize CoW strings where possible, both saving substantial amounts of memory and runtime performance penalties.

As you can see here, a Pure-Perl implementation of Clone gets this benefit automatically:

perl -MClone::PP=clone -MDevel::Peek -e 'my $x = q[ a ]; my $y = $x; print Dump $x; print Dump $y; my $z = { $y => $y }; print Dump clone $z; print Dump $y'

SV = PV(0x11d0d80) at 0x11f0d90
REFCNT = 1
FLAGS = (PADMY,POK,IsCOW,pPOK)
PV = 0x11d55d0 " a "\0
CUR = 3
LEN = 10
COW_REFCNT = 2
SV = PV(0x11d0e40) at 0x11f0cd0
REFCNT = 1
FLAGS = (PADMY,POK,IsCOW,pPOK)
PV = 0x11d55d0 " a "\0
CUR = 3
LEN = 10
COW_REFCNT = 2
SV = IV(0x11e5d50) at 0x11e5d60
REFCNT = 1
FLAGS = (TEMP,ROK)
RV = 0x11e5d48
SV = PVHV(0x11d7130) at 0x11e5d48
    REFCNT = 1
    FLAGS = (SHAREKEYS)
    ARRAY = 0x11fc090 (0:7, 1:1)
    hash quality = 100.0%
    KEYS = 1
    FILL = 1
    MAX = 7
    Elt " a " HASH = 0x5e7fc22b
    SV = PV(0x121bd70) at 0x11f0dd8
      REFCNT = 1
      FLAGS = (POK,IsCOW,pPOK)
      PV = 0x11d55d0 " a "\0
      CUR = 3
      LEN = 10
      COW_REFCNT = 4
SV = PV(0x11d0e40) at 0x11f0cd0
REFCNT = 1
FLAGS = (PADMY,POK,IsCOW,pPOK)
PV = 0x11d55d0 " a "\0
CUR = 3
LEN = 10
COW_REFCNT = 3

However, the XS implementation seems to lose the COW benefits:

perl -MClone=clone -MDevel::Peek -e 'my $x = q[ a ]; my $y = $x; print Dump $x; print Dump $y; my $z = { $y => $y }; print Dump clone $z; print Dump $y'
SV = PV(0x1dedd80) at 0x1e0dd80
REFCNT = 1
FLAGS = (PADMY,POK,IsCOW,pPOK)
PV = 0x1df25d0 " a "\0
CUR = 3
LEN = 10
COW_REFCNT = 2
SV = PV(0x1dede40) at 0x1e0dcc0
REFCNT = 1
FLAGS = (PADMY,POK,IsCOW,pPOK)
PV = 0x1df25d0 " a "\0
CUR = 3
LEN = 10
COW_REFCNT = 2
SV = IV(0x1e02c68) at 0x1e02c78
REFCNT = 1
FLAGS = (TEMP,ROK)
RV = 0x1e02ca8
SV = PVHV(0x1df4230) at 0x1e02ca8
    REFCNT = 1
    FLAGS = (SHAREKEYS)
    ARRAY = 0x1eb7330 (0:7, 1:1)
    hash quality = 100.0%
    KEYS = 1
    FILL = 1
    MAX = 7
    Elt " a " HASH = 0x729f1c06
    SV = PV(0x1deded0) at 0x1e02d38
      REFCNT = 1
      FLAGS = (POK,pPOK)
      PV = 0x1eb7380 " a "\0
      CUR = 3
      LEN = 10
SV = PV(0x1dede40) at 0x1e0dcc0
REFCNT = 1
FLAGS = (PADMY,POK,IsCOW,pPOK)
PV = 0x1df25d0 " a "\0
CUR = 3
LEN = 10
COW_REFCNT = 3

Thu Jul 31 23:57:28 2014 garu [...] cpan.org - Correspondence added

Hi Kent, thanks for the report! Sadly, I'm just doing regular maintenance of Clone and don't have the time to come up with a patch for this. However, if you can provide one and it doesn't break backwards compatibility, I'll likely merge it and push a new version to CPAN! Cheers, garu

Thu Jul 31 23:57:28 2014 The RT System itself - Status changed from 'new' to 'open'

Thu Jul 31 23:57:56 2014 garu [...] cpan.org - Severity Wishlist added

Wed Apr 05 11:58:18 2017 me [...] eboxr.com - Correspondence added

On Thu Jul 31 23:57:28 2014, GARU wrote: Show quoted text

> Hi Kent, thanks for the report! > > Sadly, I'm just doing regular maintenance of Clone and don't have the > time to come up with a patch for this. However, if you can provide one > and it doesn't break backwards compatibility, I'll likely merge it and > push a new version to CPAN! > > Cheers, > > garu

Have the feeling that the patch would be to reuse the PV one the COW flag is enabled and increase the COW_REFCNT in sv_clone for the PV case. Will try to provide a patch going in this direction

Wed Apr 05 12:12:11 2017 me [...] eboxr.com - Correspondence added

Was thinking about something like this, but this still needs some love case SVt_PV: /* 4 */ TRACEME(("string scalar\n")); #if PERL_VERSION >= 20 if ( SvIsCOW(ref) ) { /* increase the Cow refcnt if possible */ if ( CowREFCNT(ref) != SV_COW_REFCNT_MAX ) { CowREFCNT(ref)++; } clone = ref; } else { clone = newSVsv (ref); } #else clone = newSVsv (ref); #endif On Wed Apr 05 11:58:18 2017, atoomic wrote: Show quoted text

> On Thu Jul 31 23:57:28 2014, GARU wrote:

> > Hi Kent, thanks for the report! > > > > Sadly, I'm just doing regular maintenance of Clone and don't have the > > time to come up with a patch for this. However, if you can provide > > one > > and it doesn't break backwards compatibility, I'll likely merge it > > and > > push a new version to CPAN! > > > > Cheers, > > > > garu

> > Have the feeling that the patch would be to reuse the PV one the COW > flag is enabled and increase the COW_REFCNT in sv_clone for the PV > case. Will try to provide a patch going in this direction

Wed Apr 05 12:20:32 2017 me [...] eboxr.com - Correspondence added

Need to adjust some tests which are failing but seems to work with the patch> perl -Iblib/lib -Iblib/arch -MClone=clone -MDevel::Peek -e 'my $x = q[ a ]; my $y = $x; print Dump $x; print Dump $y; my $z = { $y => $y }; print Dump clone $z; print Dump $y' SV = PV(0xcbbd70) at 0xcd9868 REFCNT = 1 FLAGS = (POK,IsCOW,pPOK) PV = 0xcdd4c0 " a "\0 CUR = 3 LEN = 10 COW_REFCNT = 2 SV = PV(0xcbbe90) at 0xcd97c0 REFCNT = 1 FLAGS = (POK,IsCOW,pPOK) PV = 0xcdd4c0 " a "\0 CUR = 3 LEN = 10 COW_REFCNT = 2 SV = IV(0xccec30) at 0xccec40 REFCNT = 1 FLAGS = (TEMP,ROK) RV = 0xccec70 SV = PVHV(0xcc2360) at 0xccec70 REFCNT = 1 FLAGS = (SHAREKEYS) ARRAY = 0xcd6390 (0:7, 1:1) hash quality = 100.0% KEYS = 1 FILL = 1 MAX = 7 Elt " a " HASH = 0x761eab4b SV = PV(0xcbbeb0) at 0xccec28 REFCNT = 1 FLAGS = (POK,IsCOW,pPOK) PV = 0xcdd4c0 " a "\0 CUR = 3 LEN = 10 COW_REFCNT = 4 SV = PV(0xcbbe90) at 0xcd97c0 REFCNT = 1 FLAGS = (POK,IsCOW,pPOK) PV = 0xcdd4c0 " a "\0 CUR = 3 LEN = 10 COW_REFCNT = 3 Attempt to free unreferenced scalar: SV 0xccec28 at -e line 1 On Wed Apr 05 12:12:11 2017, atoomic wrote: Show quoted text

> Was thinking about something like this, but this still needs some love > > case SVt_PV: /* 4 */ > TRACEME(("string scalar\n")); > #if PERL_VERSION >= 20 > if ( SvIsCOW(ref) ) { > /* increase the Cow refcnt if possible */ > if ( CowREFCNT(ref) != SV_COW_REFCNT_MAX ) { > CowREFCNT(ref)++; > } > clone = ref; > } else { > clone = newSVsv (ref); > } > #else > clone = newSVsv (ref); > #endif > > On Wed Apr 05 11:58:18 2017, atoomic wrote:

> > On Thu Jul 31 23:57:28 2014, GARU wrote:

> > > Hi Kent, thanks for the report! > > > > > > Sadly, I'm just doing regular maintenance of Clone and don't have the > > > time to come up with a patch for this. However, if you can provide > > > one > > > and it doesn't break backwards compatibility, I'll likely merge it > > > and > > > push a new version to CPAN! > > > > > > Cheers, > > > > > > garu

> > > > Have the feeling that the patch would be to reuse the PV one the COW > > flag is enabled and increase the COW_REFCNT in sv_clone for the PV > > case. Will try to provide a patch going in this direction

> >

Wed Apr 05 12:28:16 2017 me [...] eboxr.com - Correspondence added

Probably better with something like this # ifdef PERL_DEBUG_READONLY_COW if (already) sv_buf_to_rw(sstr); # endif CowREFCNT(sstr)++; new_pv = SvPVX_mutable(sstr); sv_buf_to_ro(sstr); On Wed Apr 05 12:20:32 2017, atoomic wrote: Show quoted text

> Need to adjust some tests which are failing but seems to work > > with the patch> perl -Iblib/lib -Iblib/arch -MClone=clone > -MDevel::Peek -e 'my $x = q[ a ]; my $y = $x; print Dump $x; print > Dump $y; my $z = { $y => $y }; print Dump clone $z; print Dump $y' > SV = PV(0xcbbd70) at 0xcd9868 > REFCNT = 1 > FLAGS = (POK,IsCOW,pPOK) > PV = 0xcdd4c0 " a "\0 > CUR = 3 > LEN = 10 > COW_REFCNT = 2 > SV = PV(0xcbbe90) at 0xcd97c0 > REFCNT = 1 > FLAGS = (POK,IsCOW,pPOK) > PV = 0xcdd4c0 " a "\0 > CUR = 3 > LEN = 10 > COW_REFCNT = 2 > SV = IV(0xccec30) at 0xccec40 > REFCNT = 1 > FLAGS = (TEMP,ROK) > RV = 0xccec70 > SV = PVHV(0xcc2360) at 0xccec70 > REFCNT = 1 > FLAGS = (SHAREKEYS) > ARRAY = 0xcd6390 (0:7, 1:1) > hash quality = 100.0% > KEYS = 1 > FILL = 1 > MAX = 7 > Elt " a " HASH = 0x761eab4b > SV = PV(0xcbbeb0) at 0xccec28 > REFCNT = 1 > FLAGS = (POK,IsCOW,pPOK) > PV = 0xcdd4c0 " a "\0 > CUR = 3 > LEN = 10 > COW_REFCNT = 4 > SV = PV(0xcbbe90) at 0xcd97c0 > REFCNT = 1 > FLAGS = (POK,IsCOW,pPOK) > PV = 0xcdd4c0 " a "\0 > CUR = 3 > LEN = 10 > COW_REFCNT = 3 > Attempt to free unreferenced scalar: SV 0xccec28 at -e line 1 > > > On Wed Apr 05 12:12:11 2017, atoomic wrote:

> > Was thinking about something like this, but this still needs some > > love > > > > case SVt_PV: /* 4 */ > > TRACEME(("string scalar\n")); > > #if PERL_VERSION >= 20 > > if ( SvIsCOW(ref) ) { > > /* increase the Cow refcnt if possible */ > > if ( CowREFCNT(ref) != SV_COW_REFCNT_MAX ) { > > CowREFCNT(ref)++; > > } > > clone = ref; > > } else { > > clone = newSVsv (ref); > > } > > #else > > clone = newSVsv (ref); > > #endif > > > > On Wed Apr 05 11:58:18 2017, atoomic wrote:

> > > On Thu Jul 31 23:57:28 2014, GARU wrote:

> > > > Hi Kent, thanks for the report! > > > > > > > > Sadly, I'm just doing regular maintenance of Clone and don't have > > > > the > > > > time to come up with a patch for this. However, if you can > > > > provide > > > > one > > > > and it doesn't break backwards compatibility, I'll likely merge > > > > it > > > > and > > > > push a new version to CPAN! > > > > > > > > Cheers, > > > > > > > > garu

> > > > > > Have the feeling that the patch would be to reuse the PV one the > > > COW > > > flag is enabled and increase the COW_REFCNT in sv_clone for the PV > > > case. Will try to provide a patch going in this direction

> > > >

Wed Apr 05 14:44:08 2017 me [...] eboxr.com - Correspondence added

view https://github.com/garu/Clone/pull/7 for a first implementation On Wed Apr 05 12:28:16 2017, atoomic wrote: Show quoted text

> Probably better with something like this > > # ifdef PERL_DEBUG_READONLY_COW > if (already) sv_buf_to_rw(sstr); > # endif > CowREFCNT(sstr)++; > new_pv = SvPVX_mutable(sstr); > sv_buf_to_ro(sstr); > > On Wed Apr 05 12:20:32 2017, atoomic wrote:

> > Need to adjust some tests which are failing but seems to work > > > > with the patch> perl -Iblib/lib -Iblib/arch -MClone=clone > > -MDevel::Peek -e 'my $x = q[ a ]; my $y = $x; print Dump $x; print > > Dump $y; my $z = { $y => $y }; print Dump clone $z; print Dump $y' > > SV = PV(0xcbbd70) at 0xcd9868 > > REFCNT = 1 > > FLAGS = (POK,IsCOW,pPOK) > > PV = 0xcdd4c0 " a "\0 > > CUR = 3 > > LEN = 10 > > COW_REFCNT = 2 > > SV = PV(0xcbbe90) at 0xcd97c0 > > REFCNT = 1 > > FLAGS = (POK,IsCOW,pPOK) > > PV = 0xcdd4c0 " a "\0 > > CUR = 3 > > LEN = 10 > > COW_REFCNT = 2 > > SV = IV(0xccec30) at 0xccec40 > > REFCNT = 1 > > FLAGS = (TEMP,ROK) > > RV = 0xccec70 > > SV = PVHV(0xcc2360) at 0xccec70 > > REFCNT = 1 > > FLAGS = (SHAREKEYS) > > ARRAY = 0xcd6390 (0:7, 1:1) > > hash quality = 100.0% > > KEYS = 1 > > FILL = 1 > > MAX = 7 > > Elt " a " HASH = 0x761eab4b > > SV = PV(0xcbbeb0) at 0xccec28 > > REFCNT = 1 > > FLAGS = (POK,IsCOW,pPOK) > > PV = 0xcdd4c0 " a "\0 > > CUR = 3 > > LEN = 10 > > COW_REFCNT = 4 > > SV = PV(0xcbbe90) at 0xcd97c0 > > REFCNT = 1 > > FLAGS = (POK,IsCOW,pPOK) > > PV = 0xcdd4c0 " a "\0 > > CUR = 3 > > LEN = 10 > > COW_REFCNT = 3 > > Attempt to free unreferenced scalar: SV 0xccec28 at -e line 1 > > > > > > On Wed Apr 05 12:12:11 2017, atoomic wrote:

> > > Was thinking about something like this, but this still needs some > > > love > > > > > > case SVt_PV: /* 4 */ > > > TRACEME(("string scalar\n")); > > > #if PERL_VERSION >= 20 > > > if ( SvIsCOW(ref) ) { > > > /* increase the Cow refcnt if possible */ > > > if ( CowREFCNT(ref) != SV_COW_REFCNT_MAX ) { > > > CowREFCNT(ref)++; > > > } > > > clone = ref; > > > } else { > > > clone = newSVsv (ref); > > > } > > > #else > > > clone = newSVsv (ref); > > > #endif > > > > > > On Wed Apr 05 11:58:18 2017, atoomic wrote:

> > > > On Thu Jul 31 23:57:28 2014, GARU wrote:

> > > > > Hi Kent, thanks for the report! > > > > > > > > > > Sadly, I'm just doing regular maintenance of Clone and don't have > > > > > the > > > > > time to come up with a patch for this. However, if you can > > > > > provide > > > > > one > > > > > and it doesn't break backwards compatibility, I'll likely merge > > > > > it > > > > > and > > > > > push a new version to CPAN! > > > > > > > > > > Cheers, > > > > > > > > > > garu

> > > > > > > > Have the feeling that the patch would be to reuse the PV one the > > > > COW > > > > flag is enabled and increase the COW_REFCNT in sv_clone for the PV > > > > case. Will try to provide a patch going in this direction

> > > > > >

> >

Tue Oct 23 19:07:49 2018 garu [...] cpan.org - Correspondence added

Hey everyone, thank you for being so patient. Clone 0.40 is shipped! Please test and let me know if you bump into any issues \o/ On Wed Apr 05 14:44:08 2017, atoomic wrote: Show quoted text

> view https://github.com/garu/Clone/pull/7 for a first implementation > > On Wed Apr 05 12:28:16 2017, atoomic wrote:

> > Probably better with something like this > > > > # ifdef PERL_DEBUG_READONLY_COW > > if (already) sv_buf_to_rw(sstr); > > # endif > > CowREFCNT(sstr)++; > > new_pv = SvPVX_mutable(sstr); > > sv_buf_to_ro(sstr); > > > > On Wed Apr 05 12:20:32 2017, atoomic wrote:

> > > Need to adjust some tests which are failing but seems to work > > > > > > with the patch> perl -Iblib/lib -Iblib/arch -MClone=clone > > > -MDevel::Peek -e 'my $x = q[ a ]; my $y = $x; print Dump $x; print > > > Dump $y; my $z = { $y => $y }; print Dump clone $z; print Dump $y' > > > SV = PV(0xcbbd70) at 0xcd9868 > > > REFCNT = 1 > > > FLAGS = (POK,IsCOW,pPOK) > > > PV = 0xcdd4c0 " a "\0 > > > CUR = 3 > > > LEN = 10 > > > COW_REFCNT = 2 > > > SV = PV(0xcbbe90) at 0xcd97c0 > > > REFCNT = 1 > > > FLAGS = (POK,IsCOW,pPOK) > > > PV = 0xcdd4c0 " a "\0 > > > CUR = 3 > > > LEN = 10 > > > COW_REFCNT = 2 > > > SV = IV(0xccec30) at 0xccec40 > > > REFCNT = 1 > > > FLAGS = (TEMP,ROK) > > > RV = 0xccec70 > > > SV = PVHV(0xcc2360) at 0xccec70 > > > REFCNT = 1 > > > FLAGS = (SHAREKEYS) > > > ARRAY = 0xcd6390 (0:7, 1:1) > > > hash quality = 100.0% > > > KEYS = 1 > > > FILL = 1 > > > MAX = 7 > > > Elt " a " HASH = 0x761eab4b > > > SV = PV(0xcbbeb0) at 0xccec28 > > > REFCNT = 1 > > > FLAGS = (POK,IsCOW,pPOK) > > > PV = 0xcdd4c0 " a "\0 > > > CUR = 3 > > > LEN = 10 > > > COW_REFCNT = 4 > > > SV = PV(0xcbbe90) at 0xcd97c0 > > > REFCNT = 1 > > > FLAGS = (POK,IsCOW,pPOK) > > > PV = 0xcdd4c0 " a "\0 > > > CUR = 3 > > > LEN = 10 > > > COW_REFCNT = 3 > > > Attempt to free unreferenced scalar: SV 0xccec28 at -e line 1 > > > > > > > > > On Wed Apr 05 12:12:11 2017, atoomic wrote:

> > > > Was thinking about something like this, but this still needs some > > > > love > > > > > > > > case SVt_PV: /* 4 */ > > > > TRACEME(("string scalar\n")); > > > > #if PERL_VERSION >= 20 > > > > if ( SvIsCOW(ref) ) { > > > > /* increase the Cow refcnt if possible */ > > > > if ( CowREFCNT(ref) != SV_COW_REFCNT_MAX ) { > > > > CowREFCNT(ref)++; > > > > } > > > > clone = ref; > > > > } else { > > > > clone = newSVsv (ref); > > > > } > > > > #else > > > > clone = newSVsv (ref); > > > > #endif > > > > > > > > On Wed Apr 05 11:58:18 2017, atoomic wrote:

> > > > > On Thu Jul 31 23:57:28 2014, GARU wrote:

> > > > > > Hi Kent, thanks for the report! > > > > > > > > > > > > Sadly, I'm just doing regular maintenance of Clone and don't have > > > > > > the > > > > > > time to come up with a patch for this. However, if you can > > > > > > provide > > > > > > one > > > > > > and it doesn't break backwards compatibility, I'll likely merge > > > > > > it > > > > > > and > > > > > > push a new version to CPAN! > > > > > > > > > > > > Cheers, > > > > > > > > > > > > garu

> > > > > > > > > > Have the feeling that the patch would be to reuse the PV one the > > > > > COW > > > > > flag is enabled and increase the COW_REFCNT in sv_clone for the PV > > > > > case. Will try to provide a patch going in this direction

> > > > > > > >

> > > >

> >

Tue Oct 23 19:07:50 2018 garu [...] cpan.org - Status changed from 'open' to 'resolved'