develooper Front page | perl.perl5.porters | Postings from October 2016

Re: hv.h hek definition

Thread Previous | Thread Next
From:
demerphq
Date:
October 23, 2016 08:23
Subject:
Re: hv.h hek definition
Message ID:
CANgJU+X3MULDxZ24n4b3PxB31PeJn9i+8mnPUJ+iMc2P-T1rzQ@mail.gmail.com
On 22 October 2016 at 11:15, demerphq <demerphq@gmail.com> wrote:
> Personally I think the patch is probably safe with the assert added
> that bulk88 mentioned, but I kinda wonder why it wasn't done that way
> in the first place, and would want to dig a bit to see why not before
> I applied it.

It started off being at the end in a patch applied by nicholas.

I have applied your patch, merged with a version of bulk88's assert
that works if hek_flags is a char, or if it is a I32, and pushed it as
toddr/hek_flags. I have reworded the commit message, and made some
other minor changes to make it mergeable.

I tried all three ways (old code, char hek_flags, I32 hek_flags) and
observed no noticeable difference between them, but I didn't do any
rigorous benchmarking.

IMO whether this patch should be merged reduces down to answering the
following questions:

1. is an unaligned memeq() slower than an aligned one
2. what *real* effect would come from making the HEK structure 4 bytes larger

If the answer to 1 is no, then we can leave hek_flags as a char.

If the answer to 2 is "no significant difference" then we can make
hek_flags I32 and make sure that hek_key is aligned thus making the
answer to 1 irrelevant.

So the only circumstances we should not go with your patch is when
unaligned memeq is slow, and using an I32 hek_flags would grossly
inflate our memory requirements.

I suspect that the answer to one of the two questions is favourable to us.

Yves

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About