Bleadperl v5.17.3-255-g6502e08 breaks SARTAK/Path-Dispatcher-1.04.tar.gz #12395
Comments
From @andkgit bisect commit 6502e08 Don't copy all of the match string buffer diagnostics Note: Any::Moose was picking Mouse 1.02 here because current Moose [...] perl -V -- |
From @iabynOn Sun, Sep 09, 2012 at 07:31:26PM -0700, Andreas J. Koenig via RT wrote:
[snip]
This is due to this line in Path/Dispatcher/Rule/Regex.pm: which expects to be able to execute a regex where $' hasn't been seen yet, -- |
|
The RT System itself - Status changed from 'new' to 'open' |
From @cpansproutOn Mon Sep 10 08:29:17 2012, davem wrote:
I have long considered it a bug that eval '$&' doesn’t work if the three That you have extended the bug further worries me. I know you spent a lot of time on this, but it sort of dashes any hopes An idea I had, which I started to explore a year ago, but never quite That in itself might speed up the cases you sped up differently, if we See <https://rt-archive.perl.org/perl5/Ticket/Display.html?id=37038#txn-1060310>. -- Father Chrysostomos |
From p5p@sartak.orgDave Mitchell wrote:
Hi, Author of the module in question here. This line of code is, of course, simply an optimization. I have no Unfortunately grep.cpan.me seems to be down, so I can't investigate how Shawn |
From @iabynOn Mon, Sep 10, 2012 at 02:06:53PM -0400, Shawn M Moore wrote:
Rather than polluting the whole program with $', wouldn't it be better
Hopefully not too many :-( -- |
From @doyOn Mon, Sep 10, 2012 at 08:52:14PM +0100, Dave Mitchell wrote:
It's matching against user-specified regexes, so no. On the other hand, -doy |
From @iabynOn Mon, Sep 10, 2012 at 09:15:06AM -0700, Father Chrysostomos via RT wrote:
But I thought COW had too many edge cases to be viable? -- |
From dennis.kaarsemaker@booking.comOn ma, 2012-09-10 at 14:06 -0400, Shawn M Moore wrote:
Unique, it seems: [dkaarsemaker@llama ~]$ csearch "q{\\s*\\\$'\\s*}" 2>/dev/null -- |
From @iabynOn Mon, Sep 10, 2012 at 02:56:44PM -0500, Jesse Luehrs wrote:
Why not $user_regex = qr/$user_regex(.*)$/;
That would be better than nothing. -- |
From @cpansproutOn Mon Sep 10 13:02:21 2012, davem wrote:
I don’t know what you are referring to exactly. I think it is perfectly viable. If we revive the PERL_OLD_COPY_ON_WRITE • Upgrade any scalar to PVIV, or even discard a cached integer, if the On Aug 3, 2012, at 8:14 AM, Nicholas Clark wrote:
I think the biggest gain would be with large strings. If we do simple -- Father Chrysostomos |
From @cpansproutOn Mon Sep 10 15:52:14 2012, sprout wrote:
And I was going to say: This may not speed up most code in general, but -- Father Chrysostomos |
From @iabynOn Mon, Sep 10, 2012 at 03:52:15PM -0700, Father Chrysostomos via RT wrote:
From Nicholas's recent description of his COW work:
Which I read as indicating there were subtle and risky issues. -- |
From @cpansproutOn Tue Sep 11 06:04:53 2012, davem wrote:
I think this part reasonably solves that:
-- Father Chrysostomos |
From @nwc10On Mon, Sep 10, 2012 at 09:15:06AM -0700, Father Chrysostomos via RT wrote:
Me too.
But I wasn't confident of this part. After all, I fixed C<study> so that it could work on more than one string. On Tue, Sep 11, 2012 at 12:41:25PM -0700, Father Chrysostomos via RT wrote:
It still doesn't solve the test failures in one of the Compress::Zlib modules. In particular, I remember the frustration that led to this comment in /* XXX. If you make this PVIV, then copy on write can copy scalars read The swipe code seems to be very effective. However, I don't totally like it But I had a thought about an alternative COW mechanism, exploiting the To avoid moving memory around when truncating strings this permits SvPVX() SvPVX() If the flag byte is non-zero, then it's the offset in bytes back to the I wondered if that scheme can change. It stays as: SvPVX() but the flag byte is split in two in some fashion. Possibly, low 7 bits SvPVX() and is 1 if there's the rest of the OOK structure earlier: SvPVX() I think only a small amount of core code would need to change to accommodate It might be cheap enough to use everywhere when creating strings, and thus I'm also wondering if it's complementary with the other COW approach. However, I'm still wary of the Compress::Zlib test failure, as an indicator Also, I'm thinking that we ought to get rid of the (ab)use of SvREADONLY() Nicholas Clark |
From @cpansproutOn Mon Sep 17 14:06:19 2012, nicholas wrote:
Are you referring to the use of _nosteal here and there, or are you
...snip details... That is just brilliant!
That’s a lot smaller than allocating a shared_he (20 bytes on a 32-bit
I may have misunderstood, but it doesn’t sound as though your 1-byte
It is probably doing something naughty. :-)
What about existing XS code that checks SvREADONLY before modifying The flag space is rather crowded. Maybe it’s about time we made pad -- Father Chrysostomos |
From @demerphqOn 17 September 2012 23:05, Nicholas Clark <nick@ccl4.org> wrote:
You say "historically this was done with SvIVX()" and then imply not Also a note for readers new to this subject: "truncate" means "remove Yves -- |
From @nwc10On Tue, Sep 18, 2012 at 11:00:08AM +0200, demerphq wrote:
We do do the OOK hack without upgrading, as of (it seems) v5.12.0 $ perl5.10.0 -MDevel::Peek -le '$_ = "half pint"; s/half //; Dump Contrast the IV = 5 (OFFSET) with the use of "\5" to store the offset. Here's the change of storage format: $ perl -MDevel::Peek -le '$_ = "N" x 256; s/.{255}//; Dump And this is the -DDEBUGGING version being paranoid that someone might $ ./perl -Ilib -MDevel::Peek -le '$_ = "N" x 257; s/.{256}//; Dump Nicholas Clark |
From @iabynOn Mon, Sep 17, 2012 at 10:05:34PM +0100, Nicholas Clark wrote:
I have no objection to my 'copy only a bit of the buffer' code being
So this would involve any string about to be COWed to need to be Moved One thing I don't yet understand about COW (either scheme) is how if (SvPOK(sv) && SvCUR(sv)) -- |
From @nwc10On Tue, Sep 18, 2012 at 10:38:01AM +0100, Dave Mitchell wrote:
Yes. Which was why I suggested that a) one runs with both available
You can't, if they don't give an $expletive about the READONLY flag. Nicholas Clark |
From @demerphqOn 18 September 2012 12:00, Nicholas Clark <nick@ccl4.org> wrote:
Maybe a SvPVXW macro, which would check this and throw an error would Yves -- |
From @demerphqOn 18 September 2012 11:11, Nicholas Clark <nick@ccl4.org> wrote:
That is very cool. Thanks a lot for explaining, and I guess doing it. -- |
From @salvaOn 09/18/2012 11:38 AM, Dave Mitchell wrote:
There are XS modules out there that place C structures on the PV slot |
From @nwc10On Tue, Sep 18, 2012 at 12:14:45PM +0200, Salvador Fandino wrote:
Good point. I think that there are XS modules in ext/ which rely on that :-) So it would only be sane to do this for new PVs that are explicitly known Nicholas Clark |
From @ap* Nicholas Clark <nick@ccl4.org> [2012-09-18 12:05]:
Just allocate it another 1 (or even 3) bytes off on platforms where that |
From @LeontOn Tue, Sep 18, 2012 at 11:38 AM, Dave Mitchell <davem@iabyn.com> wrote:
There is already SV_CHECK_THINKFIRST (and friends) to fix all such Leon |
From @nwc10On Tue, Sep 18, 2012 at 01:00:58PM +0200, Aristotle Pagaltzis wrote:
I think that it would be 3 or 7. ["real" platforms have words that are I'm not sure that that is as worth it, given that a) the seduction of the use of 1 byte is that it's a really low memory cost, Would be the right sort of thing to benchmark to find out the costs/benefits, Nicholas Clark |
From @cpansproutOn Mon Sep 17 21:44:53 2012, sprout wrote:
This is enough to get it working: Inline Patchdiff --git a/cpan/Compress-Raw-Zlib/Zlib.xs b/cpan/Compress-Raw-Zlib/Zlib.xs
index 9f1d7a1..acaff84 100644
--- a/cpan/Compress-Raw-Zlib/Zlib.xs
+++ b/cpan/Compress-Raw-Zlib/Zlib.xs
@@ -614,7 +614,7 @@ char * string ;
croak("%s: buffer parameter is a reference to a reference",
- if (SvREADONLY(sv) && PL_curcop != &PL_compiling) SvUPGRADE(sv, SVt_PV); - if (s->flags & FLAG_CONSUME_INPUT && SvREADONLY(buf))
I did a quick audit and found numerous modules that use SvREADONLY. File::Map And it looks as though only POSIX::pselect will break if we make SvIsCOW I suggest we go ahead and do it. Some modules mishandling SvREADONLY Any code that wants to modify strings in place should be using
We could use Dave Mitchell’s free bit for now, which in Presumably SvREADONLY should also set the flag. Presumably we would -- Father Chrysostomos |
From @bulk88On Mon Sep 17 21:44:53 2012, sprout wrote:
There is the existing SVf_FAKE and SVf_READONLY for HEKs. Maybe I didn't Since the proposal as I read is it to introduce 2 different "start of In XS code I've used SVf_READONLY to protect PV buffers given to a OS Another thing, I've heard Perl allows non-Perl allocated PVs, see |
From @cpansproutOn Tue Oct 02 12:06:42 2012, bulk88 wrote:
We already do when we have a HEK already. But taking an existing string What we are looking for is a cheap way to do COW with existing strings.
The new flag would apply also to SVs containing shared HEKs. By using a new flag instead of SvREADONLY, we can make the dozen or so But no, the new flag would not be strictly necessary; just convenient.
I don’t think any of this would be broken. Basically, as long as any XS code that wants to modify a string does I can image that newSVpvn might need to behave differently depending on When XS modules use the PV pointer to store pointer-aligned binary data, I have just had another idea, a variation of the SvOOK hack, which may The chances of missing the opportunity for COW are 1 in 16, on 32-bit #include <stdlib.h> Output: Size for 1 is 16 It is probably worthwhile to go ahead and allocate the extra byte anyway -- Father Chrysostomos |
From @bulk88#include <stdlib.h>
#include <stdio.h>
#include <malloc.h>
#include <windows.h>
#ifdef _WIN64
#define HEAP_ENTRY_SHIFT 4
#else
#define HEAP_ENTRY_SHIFT 3
#endif
//32bit WinXP SP3 ONLY, MSVCR71.dll ONLY, not any other SP or OS or any other CRT
//if CRTs change, LFH might be turned on, this code does NOT support LFH heaps
//derived from REing ntdll and a app called Volatility
typedef struct _HEAP_ENTRY
{
USHORT Size;
USHORT PreviousSize;
UCHAR SmallTagIndex;
UCHAR Flags;
UCHAR UnusedBytes;
UCHAR SegmentIndex;
} HEAP_ENTRY, *PHEAP_ENTRY;
int main() {
int i;
for(i = 1; i<=256; i++) {
PHEAP_ENTRY HeapEntry;
void * ptr = malloc(i);
size_t RequestedEntrySize;
size_t ActualSize;
size_t size;
HeapEntry = (char*)ptr-(char*)(sizeof(*HeapEntry));
memset(ptr, 0xAF, i);
size = _msize(ptr);
//prove the heap header is parsed correctly, RequestedEntrySize must be equal to _msize's return
RequestedEntrySize = (HeapEntry->Size << HEAP_ENTRY_SHIFT) - HeapEntry->UnusedBytes;
ActualSize = (HeapEntry->Size << HEAP_ENTRY_SHIFT);
printf ("ptr=%X i=%X _msize=%X RevEngd req sz=%X Actual=%X\n",
ptr, i, size, RequestedEntrySize, ActualSize );
free(ptr);
}
return 0;
}
|
From @bulk88ptr=333DA0 i=1 _msize=1 RevEngd req sz=1 Actual=10 |
From @cpansproutOn Wed Oct 03 12:57:42 2012, sprout wrote:
But it causes problems in the regexp engine, which sets SvCUR to So instead I’m putting the refcount at the very end of the buffer.
Actually, what I am going to do is modify calls to sv_grow throughout
We cannot COW with any read-only scalars at all. Existing well-behaved (Unless we define SvIsCOW differently for XS.) -- Father Chrysostomos |
From @cpansproutI still have a few problems to iron out, but here come the stupid To do COW with a refcount stored in the string buffer, we have to check perl.git is the COW version. Short string: Pint:perl.git sprout$ time ./miniperl -e '$x = "hello"; $y = $x for real 0m3.137s real 0m2.597s No cow is clearly the winner. Long string: Pint:perl.git sprout$ time ./miniperl -e '$x = "hello"x1000; $y = $x real 0m3.139s real 0m7.533s For cow, the speed is the same. For no cow, things are clearly slower. Harness is hanging for some reason, but minitest works so far. ‘make You can play with it if you want. It’s on the sprout/ookow branch -- Father Chrysostomos |
From @cpansproutOn Mon Oct 08 00:37:16 2012, sprout wrote:
I forgot to mention it requires -Accflags=-DPERL_NEW_COPY_ON_WRITE. -- Father Chrysostomos |
From @cpansproutOn Mon Oct 08 08:40:33 2012, sprout wrote:
And now I have all core tests passing, except Peek.t, which will need I still need to do some benchmarks to find a threshold where I also need to allocate that extra byte for strings above the COW And then I need to see whether eliminating swipe makes any speed difference. And then I need to benchmark it against some real code. (Test.Simple’s -- Father Chrysostomos |
From [Unknown Contact. See original ticket]On Mon Oct 08 08:40:33 2012, sprout wrote:
And now I have all core tests passing, except Peek.t, which will need I still need to do some benchmarks to find a threshold where I also need to allocate that extra byte for strings above the COW And then I need to see whether eliminating swipe makes any speed difference. And then I need to benchmark it against some real code. (Test.Simple’s -- Father Chrysostomos |
From @rurbanOn Tue, Oct 2, 2012 at 11:44 PM, Father Chrysostomos via RT
Both, though only the first property is yet used. SvREADONLY data is currently only used for run-time checks, BTW: Optimizing on data properties is not a good idea. We generally optimize
No.
It is a reserved data flag to be used for compile-time and run-time checks, At those times we only implemented the simpliest readonly-ness in PHP fashion
No
Yes. readonly-less is only shallow. A HV key points to a new SV,
No No answers yet to readonly function arguments and return declarations |
From @cpansproutOn Tue Oct 16 10:14:42 2012, sprout wrote:
You can now see this on the sprout/ookow2 branch. It still requires There are two #defines, SV_COW_THRESHOLD and SV_COWBUF_THRESHOLD. These For benchmarking I compiled without -DDEBUGGING. Earlier I had used it, I tried those silly benchmarks with repeated variable assignment in a When I tried those numbers (209/2131) with mktables, it made absolutely I then tried David Wheeler’s Test.Simple JavaScript library with I set up those #defines with #ifndef conditions so that For anyone who wants to try Test.Simple with blead (I think it makes a Get JS::Test::Simple off CPAN and then move the tests/ folder somewhere Install Lexical::Var into your blead installation with the patch from Install WWW::Scripter::Plugin::Ajax. Then run this ‘one’-liner, with the right file: URL: bleadperl -MTime::HiRes=sleep -e '++$INC{"JE/Destroyer.pm"};use The %INC fiddling is a necessary hack. It basically has to do with
Not done yet, but that is next on my list.
Yes. It makes things slower. (And, smueller, did you see my message about smoking sprout/ookow? If -- Father Chrysostomos |
From chromatic@wgz.orgOn Tuesday, October 23, 2012 01:01:56 PM Father Chrysostomos via RT wrote:
FWIW, that's been the bottleneck in most of the microbenchmarks I've profiled -- c |
From @cpansproutOn Tue Oct 23 13:01:56 2012, sprout wrote:
*new*
s/of\K/f/
I forgot to mention that the speed increase I saw with mktables was Those might not be significant, but programs that deal with large -- Father Chrysostomos |
From @tseeOn 10/23/2012 10:01 PM, Father Chrysostomos via RT wrote:
Umm, too late. I think you missed my mail about starting the smoke. It http://users.perl5.git.perl.org/~tsee/sprout-ookow/ (Progress: http://users.perl5.git.perl.org/~tsee/progress.html) Should I kick off sprout/ookow2 nonetheless? --Steffen |
From @cpansproutOn Tue Oct 23 13:08:11 2012, smueller@cpan.org wrote:
Thank you. But, I think you missed my follow-up message. In which case it is my (Or am I misreading the reports?)
On second thought, sprout/ookow (without the 2) is a better robustness Would you mind doing another sprout/ookow smoke (with BTW, perl -V output should show PERL_NEW_COPY_ON_WRITE under -- Father Chrysostomos |
From @cpansproutAnd in case the changed subject confused anyone (I wish RT could keep On Tue Oct 23 13:53:19 2012, sprout wrote:
-- Father Chrysostomos |
From @cpansproutOn Tue Oct 23 13:01:56 2012, sprout wrote:
It is giving me a bizarre result. Just making the SvGROW macro use That’s probably happening because I added extra code to a macro used in So a ‘cow when we can’ approach, without making ‘we can’ occur more -- Father Chrysostomos |
From [Unknown Contact. See original ticket]On Tue Oct 23 13:01:56 2012, sprout wrote:
It is giving me a bizarre result. Just making the SvGROW macro use That’s probably happening because I added extra code to a macro used in So a ‘cow when we can’ approach, without making ‘we can’ occur more -- Father Chrysostomos |
From @tseeOn 10/23/2012 10:54 PM, Father Chrysostomos via RT wrote:
Consider it done. --Steffen |
From @cpansproutOn Tue Oct 23 16:49:40 2012, sprout wrote:
My next step after that was going to be to enable COW for magic values. COW is currently disabled even when assigning a shared hash key scalar #define CAN_COW_MASK (SVs_OBJECT|SVs_GMG|SVs_SMG|SVs_RMG|SVf_IOK|SVf_NOK| \ I cannot see how blessedness or magic can affect COWs. Most of the This punting on COW for shared hash key scalars was introduced by this commit b8f9541 Ensure string table counts are balanced. (Was not true in op/pack.t) One commit before that, I see no unbalanced string table warnings with I imagine SVt_BREAK must have been the cause, but it doesn’t happen on If I hear no objection soon, I will change it simply to an SVf_BREAK test. (And if Nicholas Clark sees a problem, he still has several months to -- Father Chrysostomos |
From [Unknown Contact. See original ticket]On Tue Oct 23 16:49:40 2012, sprout wrote:
My next step after that was going to be to enable COW for magic values. COW is currently disabled even when assigning a shared hash key scalar #define CAN_COW_MASK (SVs_OBJECT|SVs_GMG|SVs_SMG|SVs_RMG|SVf_IOK|SVf_NOK| \ I cannot see how blessedness or magic can affect COWs. Most of the This punting on COW for shared hash key scalars was introduced by this commit b8f9541 Ensure string table counts are balanced. (Was not true in op/pack.t) One commit before that, I see no unbalanced string table warnings with I imagine SVt_BREAK must have been the cause, but it doesn’t happen on If I hear no objection soon, I will change it simply to an SVf_BREAK test. (And if Nicholas Clark sees a problem, he still has several months to -- Father Chrysostomos |
From @cpansproutOn Thu Oct 04 05:17:48 2012, davem wrote:
I have a commit on the sprout/ookow2 branch that adds and uses the -- Father Chrysostomos |
From @cpansproutOn Tue Oct 23 22:44:44 2012, smueller@cpan.org wrote:
Apologies for taking so long to respond to say thank you. This is I see there are only 29 failures, some of which are due to an Encode bug -- Father Chrysostomos |
From [Unknown Contact. See original ticket]On Tue Oct 23 22:44:44 2012, smueller@cpan.org wrote:
Apologies for taking so long to respond to say thank you. This is I see there are only 29 failures, some of which are due to an Encode bug -- Father Chrysostomos |
From @cpansproutOn Fri Oct 26 18:08:02 2012, sprout wrote:
I’ve merged my patch as e3918bb, but the commit message has a mistake -- Father Chrysostomos |
From [Unknown Contact. See original ticket]On Fri Oct 26 18:08:02 2012, sprout wrote:
I’ve merged my patch as e3918bb, but the commit message has a mistake -- Father Chrysostomos |
From @cpansproutOn Mon Sep 10 08:29:17 2012, davem wrote:
It does work as of 1a904fc. -- Father Chrysostomos |
|
@cpansprout - Status changed from 'open' to 'resolved' |
From @cpansproutOn Thu Oct 04 16:33:10 2012, bulk88 wrote:
http://perl5.git.perl.org/perl.git/blob/6aa4fbb50135d4863535200030aadfeb8c583c6e:/win32/vmem.h#l207
It’s interesting to see that it allocates memory in increments of 8 I wonder, would Windows benefit from different COW threshold settings I have the #defines set up such that defines on the command line will If someone with access to Windows could try different values for -- Father Chrysostomos |
From [Unknown Contact. See original ticket]On Thu Oct 04 16:33:10 2012, bulk88 wrote:
http://perl5.git.perl.org/perl.git/blob/6aa4fbb50135d4863535200030aadfeb8c583c6e:/win32/vmem.h#l207
It’s interesting to see that it allocates memory in increments of 8 I wonder, would Windows benefit from different COW threshold settings I have the #defines set up such that defines on the command line will If someone with access to Windows could try different values for -- Father Chrysostomos |
Migrated from rt.perl.org#114820 (status was 'resolved')
Searchable as RT114820$
The text was updated successfully, but these errors were encountered: