Skip to content

Commit

Permalink
Use a flat array for line coverage.
Browse files Browse the repository at this point in the history
  • Loading branch information
Taavi Burns committed Feb 9, 2012
1 parent 004b99c commit cae8255
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 49 deletions.
93 changes: 50 additions & 43 deletions xdebug_code_coverage.c
Expand Up @@ -26,18 +26,45 @@

extern ZEND_DECLARE_MODULE_GLOBALS(xdebug);

void xdebug_coverage_line_dtor(void *data)
inline void xdebug_coverage_lines_alloc(xdebug_coverage_file *file)
{
xdebug_coverage_line *line = (xdebug_coverage_line *) data;
file->lines_slots = 64;
file->lines = (xdebug_coverage_line *) xdmalloc(
file->lines_slots * sizeof(xdebug_coverage_line));

This comment has been minimized.

Copy link
@derickr

derickr Feb 16, 2012

There is no need to break this line up in two. The rest of the code isn't shy of longer lines either.

This comment has been minimized.

Copy link
@taavi

taavi Feb 16, 2012

Owner

Sure, will do.

memset(
file->lines,
0,
file->lines_slots * sizeof(xdebug_coverage_line));
}

inline void xdebug_coverage_lines_dealloc(xdebug_coverage_file *file)
{
file->lines_slots = 0;
xdfree(file->lines);
file->lines = NULL;
}

xdfree(line);
inline xdebug_coverage_line* xdebug_coverage_lines_get(xdebug_coverage_file *file, int lineno) {
int new_size = file->lines_slots;
while (lineno >= new_size) {
new_size *= 2;
}
if (new_size > file->lines_slots) {
file->lines = (xdebug_coverage_line *) xdrealloc(file->lines, new_size * sizeof(xdebug_coverage_line));
memset(
&(file->lines[file->lines_slots]),
0,
(new_size - file->lines_slots) * sizeof(xdebug_coverage_line));
file->lines_slots = new_size;
}
return &(file->lines[lineno]);
}

void xdebug_coverage_file_dtor(void *data)
{
xdebug_coverage_file *file = (xdebug_coverage_file *) data;

xdebug_hash_destroy(file->lines);
xdebug_coverage_lines_dealloc(file);
xdfree(file->name);
xdfree(file);
}
Expand Down Expand Up @@ -286,23 +313,17 @@ void xdebug_count_line(char *filename, int lineno, int executable, int deadcode
* add a line element to the file */
file = xdmalloc(sizeof(xdebug_coverage_file));
file->name = xdstrdup(filename);
file->lines = xdebug_hash_alloc(128, xdebug_coverage_line_dtor);
xdebug_coverage_lines_alloc(file);

xdebug_hash_add(XG(code_coverage), filename, strlen(filename), file);
}
XG(previous_filename) = file->name;
XG(previous_file) = file;
}

/* Check if the line already exists in the hash */
if (!xdebug_hash_index_find(file->lines, lineno, (void *) &line)) {
line = xdmalloc(sizeof(xdebug_coverage_line));
line->lineno = lineno;
line->count = 0;
line->executable = 0;

xdebug_hash_index_add(file->lines, lineno, line);
}
/* Get the address to the line struct (will realloc if necessary) */
line = xdebug_coverage_lines_get(file, lineno);
line->hit = 1;

if (executable) {
if (line->executable != 1 && deadcode) {
Expand All @@ -311,7 +332,7 @@ void xdebug_count_line(char *filename, int lineno, int executable, int deadcode
line->executable = 1;
}
} else {
line->count++;
line->count = 1;

This comment has been minimized.

Copy link
@derickr

derickr Feb 16, 2012

Wouldn't this change behaviour?

This comment has been minimized.

Copy link
@taavi

taavi Feb 16, 2012

Owner

It changes the data stored, but not what the outside world sees (assuming I understand this file properly).

As far as I can tell, the only way that this coverage data gets out is via add_lines() (formerly xdebug_hash_apply(…, add_line)).

Old line 625 returned "1" if the line got coverage, never the number of times it was hit. Its equivalent (on new line 634) preserves that behaviour. Hence, I don't think any callers will notice, and it means that line->count fits in 1 bit instead of requiring something on the order of 32bits. :)

Does that logic hold?

}
}

Expand Down Expand Up @@ -598,31 +619,21 @@ PHP_FUNCTION(xdebug_stop_code_coverage)
RETURN_FALSE;
}


static int xdebug_lineno_cmp(const void *a, const void *b TSRMLS_DC)
static void add_lines(void *ret, xdebug_coverage_file *file)
{
Bucket *f = *((Bucket **) a);
Bucket *s = *((Bucket **) b);

if (f->h < s->h) {
return -1;
} else if (f->h > s->h) {
return 1;
} else {
return 0;
}
}


static void add_line(void *ret, xdebug_hash_element *e)
{
xdebug_coverage_line *line = (xdebug_coverage_line*) e->ptr;
zval *retval = (zval*) ret;
long lineno;
xdebug_coverage_line *line;

if (line->executable && (line->count == 0)) {
add_index_long(retval, line->lineno, -line->executable);
} else {
add_index_long(retval, line->lineno, 1);
for (lineno = 0; lineno < file->lines_slots; lineno++) {

This comment has been minimized.

Copy link
@derickr

derickr Feb 16, 2012

Can the line no really be 0? I thought they started at 1.

This comment has been minimized.

Copy link
@taavi

taavi Feb 16, 2012

Owner

True. The 0th element should never have anything in it anyway; I can start counting from 1.

line = &file->lines[lineno];
if (line->hit) {
if (line->executable && (line->count == 0)) {
add_index_long(retval, lineno, -line->executable);
} else {
add_index_long(retval, lineno, 1);
}
}
}
}

Expand All @@ -637,12 +648,8 @@ static void add_file(void *ret, xdebug_hash_element *e)
MAKE_STD_ZVAL(lines);
array_init(lines);

/* Add all the lines */
xdebug_hash_apply(file->lines, (void *) lines, add_line);

/* Sort on linenumber */
target_hash = HASH_OF(lines);
zend_hash_sort(target_hash, zend_qsort, xdebug_lineno_cmp, 0 TSRMLS_CC);
/* Add all the lines (in order by definition) */
add_lines((void *) lines, file);

add_assoc_zval_ex(retval, file->name, strlen(file->name) + 1, lines);
}
Expand Down
12 changes: 6 additions & 6 deletions xdebug_code_coverage.h
Expand Up @@ -24,14 +24,15 @@
#include "xdebug_mm.h"

typedef struct xdebug_coverage_line {
int lineno;
int count;
int executable;
unsigned int hit:1;
unsigned int count:1;
unsigned int executable:2;
} xdebug_coverage_line;

typedef struct xdebug_coverage_file {
char *name;
xdebug_hash *lines;
char *name;
long lines_slots;
xdebug_coverage_line *lines;
} xdebug_coverage_file;

/* Needed for code coverage as Zend doesn't always add EXT_STMT when expected */
Expand All @@ -41,7 +42,6 @@ typedef struct xdebug_coverage_file {
zend_set_user_opcode_handler(oc, xdebug_##f##_handler);


void xdebug_coverage_line_dtor(void *data);
void xdebug_coverage_file_dtor(void *data);

int xdebug_common_override_handler(ZEND_OPCODE_HANDLER_ARGS);
Expand Down

0 comments on commit cae8255

Please sign in to comment.