op_seq (was: Freeing code)

[Paul Johnson]

Here's something I've had on my TODO list for slightly over a year. I
sent a test patch last year, fishing for comments, but no one bit. So
here's the real thing.

op_seq uses sixteen bits, as does op_type. op_seq only needs to hold
three states, and op_type only needs nine bits. This means that we
could store all the information we need in eleven bits, save sixteen
bits per op and still have five bits spare for future use.

The attached patch does just that. All the tests still pass on my i386
linux box and the memory requirements of the following program are
reduced by almost 2.5%.

use POSIX;
use CGI;
use IO;
use Encode;
use Storable;
use Switch;
use Getopt::Long;

Obviously the reduction in memory usage depends on the proportion of ops
to other data.

Notes:

- perl regen.pl needs to be run. In fact, it needs to be run anyway,
whether or not this patch is applied.
- Of course, binary compatibility is completely shot.
- So is Devel::Cover. I'm not aware of any other CPAN modules which use
op_seq, but there probably are some.
- Fake sequence numbers are provided for -Dx output, in a similar
fashion to that which Stephen McCamant implemented for B::Concise.
The order of the numbers assigned to the ops, and which ops are
assigned sequence numbers, results in slightly altered -Dx output.
This doesn't seem important to me, but extra work could be put into
the dump routines to make the output more similar. Or some of the
complexity could be removed if slightly greater changes were
acceptable.
- I can detect no change in execution speed.
- perlcc works as well as it did before.
- 88 of the test.bytecode target tests fail. This is the same as before.
- The bytecode still stores op_type in sixteen bits and each of op_opt
and op_static in eight bits. This could obviously be improved. Does
anyone still care about this?

--
Paul Johnson - pa...@pjcj.net
http://www.pjcj.net

[Rafael Garcia-Suarez]

Paul Johnson wrote:
>
> Here's something I've had on my TODO list for slightly over a year. I
> sent a test patch last year, fishing for comments, but no one bit. So
> here's the real thing.
>
> op_seq uses sixteen bits, as does op_type. op_seq only needs to hold
> three states, and op_type only needs nine bits. This means that we
> could store all the information we need in eleven bits, save sixteen
> bits per op and still have five bits spare for future use.
>
> The attached patch does just that.

Thanks, applied as #22353.
I bumped up the version number of the various impacted B:: modules,
due to API changes (no more op_seq method for B::OP.)

> Obviously the reduction in memory usage depends on the proportion of ops
> to other data.
>
> Notes:
>
> - perl regen.pl needs to be run. In fact, it needs to be run anyway,
> whether or not this patch is applied.
> - Of course, binary compatibility is completely shot.

It was shot already :)

> - So is Devel::Cover. I'm not aware of any other CPAN modules which use
> op_seq, but there probably are some.

One of those days I'll install that grepcpan script.

[Nicholas Clark]

On Sat, Feb 21, 2004 at 02:31:47AM +0100, Paul Johnson wrote:
> Here's something I've had on my TODO list for slightly over a year. I
> sent a test patch last year, fishing for comments, but no one bit. So
> here's the real thing.
>
> op_seq uses sixteen bits, as does op_type. op_seq only needs to hold
> three states, and op_type only needs nine bits. This means that we
> could store all the information we need in eleven bits, save sixteen
> bits per op and still have five bits spare for future use.

You don't mention in this summary that your patch then (appears to) use that
saved space for:

+ * op_opt Whether or not the op has been optimised by the
+ * peephole optimiser.
+ * op_static Whether or not the op is statically defined.
+ * This flag is used by the B::C compiler backend
+ * and indicates that the op should not be freed.

Which confuses me because then you report:

> The attached patch does just that. All the tests still pass on my i386
> linux box and the memory requirements of the following program are
> reduced by almost 2.5%.

This is good. There won't be memory reduction on any platform which requires
structs containing 4 byte values to be 4 byte aligned, so it's not going to
be a universal saving.

Nicholas Clark

[Stephen McCamant]

>>>>> "NC" == Nicholas Clark <ni...@ccl4.org> writes:

NC> On Sat, Feb 21, 2004 at 02:31:47AM +0100, Paul Johnson wrote:

PJ> op_seq uses sixteen bits, as does op_type. op_seq only needs to
PJ> hold three states, and op_type only needs nine bits. This means
PJ> that we could store all the information we need in eleven bits,
PJ> save sixteen bits per op and still have five bits spare for future
PJ> use.

NC> You don't mention in this summary that your patch then (appears
NC> to) use that saved space for:

NC> + * op_opt Whether or not the op has been optimised by the
NC> + * peephole optimiser.
NC> + * op_static Whether or not the op is statically defined.
NC> + * This flag is used by the B::C compiler backend
NC> + * and indicates that the op should not be freed.

These are the two bits that hold the three states that op_seq used
to. !op_opt corresponds to op_seq==0 and op_static to op_seq=65535.

NC> Which confuses me because then you report:

PJ> The attached patch does just that. All the tests still pass on my
PJ> i386 linux box and the memory requirements of the following
PJ> program are reduced by almost 2.5%.

NC> This is good. There won't be memory reduction on any platform
NC> which requires structs containing 4 byte values to be 4 byte
NC> aligned, so it's not going to be a universal saving.

Eh? It looks to me like this change will save 4 bytes per OP on
platforms where OPs are 4-byte-aligned, by reducing BASEOP from 24
bytes to 20. Assuming 32 bit pointers, this changes BASEOP from

pointer
--
pointer
--
pointer
--
U32
--
U16
U16
--
U8
U8
<16 wasted bits>

to

pointer
--
pointer
--
pointer
--
U32
--
9 bits
1 bit
1 bit
5 bits

This may not all be achieved if malloc() aligns to 8 bytes, but some
OPs have an even number of additional pointers, while others have an
odd number, so some will see a win.

-- Stephen

[Nicholas Clark]

On Mon, Feb 23, 2004 at 05:26:17PM -0800, Stephen McCamant wrote:
> >>>>> "NC" == Nicholas Clark <ni...@ccl4.org> writes:

> NC> Which confuses me because then you report:

> Eh? It looks to me like this change will save 4 bytes per OP on

> platforms where OPs are 4-byte-aligned, by reducing BASEOP from 24
> bytes to 20. Assuming 32 bit pointers, this changes BASEOP from

I know now where my confusion came from. I saw a patch hunk adding U8s
which was actually functions (or macros) to access the structure.
The struct itself (and its changes) were in a patch hunk mutch later on.

Any reason not to convert all the op_flags and op_private elements to
bitfields in a struct, and remove a lot of anding with OPf_REF etc

Mmm. It looks like op_private is too overloaded for this to be useful.

Nicholas Clark