Control: tag -1 patch On Tue, Dec 29, 2015 at 10:29:36AM +0200, Niko Tyni wrote:
> I've made some progress on it fwiw, and it seems to be something with > two print statements in different modules getting the same opcode address > on the perl side, therefore hashing down to a same hash key and getting > counted as one. Will update the upstream bug when I've got.to the bottom > of it. I'm attaching the patch I've just sent upstream, but I suggest we wait a bit for Paul to comment rather than apply it for Debian straight away. I'm sure it could be better :) -- Niko Tyni [email protected]
>From 80ea58445e6489c62e841ca8392cc3a415daf525 Mon Sep 17 00:00:00 2001 From: Niko Tyni <[email protected]> Date: Wed, 30 Dec 2015 00:49:47 +0200 Subject: [PATCH] Use file location information for coverage calculations The coverage calculations rely on 'struct unique' being unique between Perl ops. It turns out that the Perl memory manager is getting better at reusing memory for similar opcodes, resulting in hash key collisions in some cases for different but very similar statements. Adding the file name and location to the mix where available ("cops", i.e. OP_NEXTSTATE and OP_DBSTATE) fixes this. We compute a fixed size hash of the information using a Fowler/Noll/Vo (FNV) hash function, which is dead simple and public domain. Bug-Debian: https://bugs.debian.org/800424 Bug: https://github.com/pjcj/Devel--Cover/issues/143 --- Cover.xs | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/Cover.xs b/Cover.xs index 47a6064..2411f12 100644 --- a/Cover.xs +++ b/Cover.xs @@ -66,6 +66,8 @@ extern "C" { struct unique { /* Well, we'll be fairly unlucky if it's not */ OP *addr, op; + /* include hashed file location information, where available (cops) */ + size_t fileinfohash; }; #define KEY_SZ sizeof(struct unique) @@ -184,13 +186,50 @@ static int cpu() { #endif /* HAS_GETTIMEOFDAY */ +/* + * http://codereview.stackexchange.com/questions/85556/simple-string-hashing-algorithm-implementation + * https://en.wikipedia.org/wiki/Fowler%E2%80%93Noll%E2%80%93Vo_hash_function + * http://www.isthe.com/chongo/tech/comp/fnv/index.html#public_domain + * + * FNV hash algorithms and source code have been released into the + * public domain. The authors of the FNV algorithmm took deliberate + * steps to disclose the algorhtm in a public forum soon after it was + * invented. More than a year passed after this public disclosure and the + * authors deliberatly took no steps to patent the FNV algorithm. Therefore + * it is safe to say that the FNV authors have no patent claims on the FNV + * algorithm as published. + * +*/ + +/* Fowler/Noll/Vo (FNV) hash function, variant 1a */ +static size_t fnv1a_hash(const char* cp) +{ + size_t hash = 0x811c9dc5; + while (*cp) { + hash ^= (unsigned char) *cp++; + hash *= 0x01000193; + } + return hash; +} + +#define FILEINFOSZ 1024 + static char *get_key(OP *o) { static struct unique uniq; + static char mybuf[FILEINFOSZ]; uniq.addr = o; uniq.op = *o; uniq.op.op_ppaddr = 0; /* we mess with this field */ uniq.op.op_targ = 0; /* might change */ + if (o->op_type == OP_NEXTSTATE || o->op_type == OP_DBSTATE) { + /* cop, has file location information */ + snprintf(mybuf, FILEINFOSZ-1, "%s:%d", ((COP *)o)->cop_file, ((COP *)o)->cop_line); + uniq.fileinfohash = fnv1a_hash(mybuf); + } else { + /* no file location information available */ + uniq.fileinfohash = 0; + } return (char *)&uniq; } -- 2.6.4
