hv.h: rework HEK_FLAGS to a proper member in struct hek
authorTodd Rinaldo <toddr@cpan.org>
Fri, 14 Oct 2016 03:38:31 +0000 (22:38 -0500)
committerYves Orton <demerphq@gmail.com>
Mon, 24 Oct 2016 15:36:21 +0000 (17:36 +0200)
Move the store of HEK_FLAGS off the end of the allocated
hek_key into the hek struct, simplifying access and providing
clarity to the code.

What is not clear is why Nicholas or perhaps Jarkko did
not do this themselves. We use similar tricks elsewhere,
so perhaps it was just continuing a tradition...

One thought is that we often have do strcmp/memeq on these
strings, and having their start be aligned might improve
performance, wheras this patch changes them to be unaligned.
If so perhaps we should just make flags a U32 and let the
HEK's be larger. They are shared in PL_strtab, and are
probably often sitting in malloc blocks that are sufficiently
large enough that making them bigger would make no practical
difference. (All of this is worth checking.)

[with edits by Yves Orton]

hv.c
hv.h
perl.c

diff --git a/hv.c b/hv.c
index 7d9579c..338b17e 100644 (file)
--- a/hv.c
+++ b/hv.c
@@ -80,8 +80,7 @@ S_save_hek_flags(const char *str, I32 len, U32 hash, int flags)
     HEK *hek;
 
     PERL_ARGS_ASSERT_SAVE_HEK_FLAGS;
-
-    Newx(k, HEK_BASESIZE + len + 2, char);
+    Newx(k, HEK_BASESIZE + len + 1, char);
     hek = (HEK*)k;
     Copy(str, HEK_KEY(hek), len, char);
     HEK_KEY(hek)[len] = 0;
diff --git a/hv.h b/hv.h
index ee536f0..16634b7 100644 (file)
--- a/hv.h
+++ b/hv.h
@@ -45,10 +45,9 @@ struct he {
 struct hek {
     U32                hek_hash;       /* hash of key */
     I32                hek_len;        /* length of hash key */
+    char        hek_flags;      /* The flags associated with this key */
     char       hek_key[1];     /* variable-length hash key */
     /* the hash-key is \0-terminated */
-    /* after the \0 there is a byte for flags, such as whether the key
-       is UTF-8 */
 };
 
 struct shared_he {
@@ -397,7 +396,7 @@ C<SV*>.
 #define HEK_HASH(hek)          (hek)->hek_hash
 #define HEK_LEN(hek)           (hek)->hek_len
 #define HEK_KEY(hek)           (hek)->hek_key
-#define HEK_FLAGS(hek) (*((unsigned char *)(HEK_KEY(hek))+HEK_LEN(hek)+1))
+#define HEK_FLAGS(hek)                (hek)->hek_flags
 
 #define HVhek_UTF8     0x01 /* Key is utf8 encoded. */
 #define HVhek_WASUTF8  0x02 /* Key is bytes here, but was supplied as utf8. */
diff --git a/perl.c b/perl.c
index aa7d8b6..8fca566 100644 (file)
--- a/perl.c
+++ b/perl.c
@@ -322,6 +322,18 @@ perl_construct(pTHXx)
     }
 #endif /* #if defined(USE_HASH_SEED) || defined(USE_HASH_SEED_EXPLICIT) */
 
+    /* Afaik, this is good as anywhere else to assert
+     * that our HEK structure is properly sized.
+     * We expect that the char used for the flags is
+     * not padded and the bytes for the key continue right
+     * after. (For now, perhaps we should just make the flags
+     * a U32, and leave the beginning of key buffer aligned)
+     * - Yves
+     */
+    STATIC_ASSERT_STMT(  sizeof(((struct hek *)0)->hek_flags) ==
+                         (offsetof(struct hek, hek_key)
+                          - offsetof(struct hek, hek_flags)));
+
     /* Note that strtab is a rather special HV.  Assumptions are made
        about not iterating on it, and not adding tie magic to it.
        It is properly deallocated in perl_destruct() */