[P5P] [perl #78742] eval '__PACKAGE__' works differently with threads than without

Ad B:
Since I have to recreate the new PL_stashpad in B::C, Byteloader and
friends I need B::COP::stashoff exposed.

Having reliable public readers and writers for B::COP::stash would also
help, but I don't trust p5p's reliability on such low-level APIs.
Now the string API is gone and we have hashes again.
I rather do it by myself, as p5p tends to break the low-level APIs very
often and refuse to fix it later.

What about CopSTASH_set, CopSTASH_len and CopSTASH_len_set? They were
public API's from cop.h.

Ad stashpad:
Why a new pad anyway? Why not keep the stash names in PL_strtab
(sharedpvn) with threads?

stashoff is faster, indeed, but I assume also more complicate to handle.
On comparison: The similar handled regex_padav is still broken threaded
in the compiler, it should really be moved to the REGEXP body.

You are being awfully vague. I’m perfectly willing to accommodate the
needs of B::C, but I don’t understand what you need.

Does B::C need access to all of perl’s innards, such that we can’t even
fix bugs?
They were only ‘public’ in the sense that they had to be accessible for
the CopSTASH macro to work. They were never documented in perlapi.
Using the lexical pad results in very fragile code, because PL_curstash
has to be set before CopSTASH can be used.
Using strings at all causes bugs like the one quoted at the beginning of
this message.
How is it broken?

I never said that. On the contrary I would like to get bug fixed,
but they are typically warnocked for about two major releases until
they get fixed.

I said that I need either a reliable public API to read+write new data,
or expose the innards so that I can do it by myself.
There's also no need for a deprecation cycle for those innards.
I'm fine with that since I don't need them.
But others with the expectation of deprecation cycles might need them.
I see, agreed.
I don't know yet.
my_perl->regex_padav gets overwritten in the first call to Perl_call_pv@plt
which looks like loading a shared library section. I haven't
disassembled it yet.
Logically I thought it has something to do with body arena init, but
it does not look like so.
It is compiler independent and repro. The rest of my_perl seems to be
not affected.
http://code.google.com/p/perl-compiler/issues/detail?id=100

stashpad seems very alike so I'm sceptical. We'll see.

eval
eval
method.
offset
If I add stashoff to B::COP, is that sufficient for your needs?

It can simply return 0 for non-threaded perls.

And we can backport to 5.16.1 your patch that added B::COP::stashlen, in
which case we should add it back to bleadperl, even if it is no longer
necessary.

Does that solve the issues?

Yes.
I already check threaded, so no need.
I closely follow cop.h. All the struct cop fields must be exposed.
cop_stashoff is threaded only.
BTW: the comment for
PADOFFSET cop_stashoff; /* package line was compiled in */
is wrong. PL_stashpad offset it should be.
If it's not needed I don't need it either.

I have now added the exception that CopSTASHPV_set threaded needs 3
args on 5.16.0 only,
all other versions need the old and new 2 args.

I have just added it in commit a60c099b83.

The more the think about the stashpad the more I think it's worse then before.

As analog we have a regex_padav, and each PMOP stores an offset into
that for threaded perls.
The improvement would be to get rid of this interpreter-global array
and store the padav in the
new REGEXP object itself.

For stashpad's which were previously stored shared amongst threads in
strtab, they are
now stored thread-private in these stashpad's, which means that each
thread has to copy around a lot of data.
stash pv's are not likely to be seperate for each thread, the strtab
is a much better location for that.
Also has the strtab the hek infrastructure already to store len and
utf8 flag efficiently, in contrary to the new stashpad.

I propose to revert the recent stashpad changes and just use the
cop_stashlen from the hek.
No need to store this in the COP itself.

Actually, there was a separate PV allocated for *every single* COP -- a
huge waste of memory.
A lot less than before. Bear in mind that multiple COPs compiled into
the same package share the same stashpad slot. So that’s 4 octets per
package on a 32-bit architecture.
As, I said before, storing the package name (whether as a separate PV
owned by the COP, or a shared HEK), rather than a pointer to the stash
itself, causes bugs.

What I failed to mention is that PL_stashpad contains, not strings, but
stash pointers.

Oh, that makes a difference! Sorry. I've read over it somehow in the sources.

It’s in alloccopstash in op.c:

PL_stashpad[PL_stashpadix = off] = hv;

How could that possibly work? The whole point of the regex_padav is to
allow a (shared-across-threads) PMOP to retrieve a (per-thread) REGEXP
object notionally attached to it.

You would need to have the REGEXP to retrieve the regex_padav in order to
be able to, err,..., retrieve the REGEXP.

PS my desire, if possible, is to get rid of PL_regex_padav completely and
just use the standard pad; in the same way that other things like GVs are
moved from ops into the pad for threaded builds.

Since the REGEXPs are now standalone SVs, yes.

@Father:
I've got one more problem with CopSTASHPV_set:
alloccopstash needs to be properly exported. I need to call CopSTASHPV_set.

order to
completely and
GVs are
CopSTASHPV_set.

I’ve taken care of that in c0b8aebd32.