Skip to content

Commit

Permalink
Fix profiler's call graph node getting missed by the GC
Browse files Browse the repository at this point in the history
When the NodeWorklist hit its allocated size, add_node would reallocate
the list but then not actually add the node to the resized list. This
made us miss this node during the GC's mark run and led to outdated
object pointers in the allocation list.

Fix by restructuring the code, so we reallocate first and then add the
item. I also removed the unused code for handling start > 0 as was an
optimization for a condition that was never actually implemented. This
code an the added complexity probably led to the oversight in the first
place.
  • Loading branch information
niner committed Oct 2, 2019
1 parent 1546c8c commit 81f9ccd
Showing 1 changed file with 6 additions and 18 deletions.
24 changes: 6 additions & 18 deletions src/profiler/instrument.c
Expand Up @@ -2,40 +2,29 @@

typedef struct {
MVMuint32 items;
MVMuint32 start;
MVMuint32 alloc;

MVMProfileCallNode **list;
} NodeWorklist;

static void add_node(MVMThreadContext *tc, NodeWorklist *list, MVMProfileCallNode *node) {
if (list->start + list->items + 1 < list->alloc) {
/* Add at the end */
list->items++;
list->list[list->start + list->items] = node;
} else if (list->start > 0) {
/* End reached, add to the start now */
list->start--;
list->list[list->start] = node;
} else {
if (list->items == list->alloc) {
/* Filled up the whole list. Make it bigger */
list->alloc *= 2;
list->list = MVM_realloc(list->list, list->alloc * sizeof(MVMProfileCallNode *));
}
/* Add at the end */
list->list[list->items] = node;
list->items++;
}

static MVMProfileCallNode *take_node(MVMThreadContext *tc, NodeWorklist *list) {
MVMProfileCallNode *result = NULL;
if (list->items == 0) {
MVM_panic(1, "profiler: tried to take a node from an empty node worklist");
}
if (list->start > 0) {
result = list->list[list->start];
list->start++;
} else {
result = list->list[list->start + list->items];
list->items--;
}
list->items--;
result = list->list[list->items];
return result;
}

Expand Down Expand Up @@ -846,7 +835,6 @@ void MVM_profile_instrumented_mark_data(MVMThreadContext *tc, MVMGCWorklist *wor
/* Allocate our worklist on the stack. */
NodeWorklist nodelist;
nodelist.items = 0;
nodelist.start = 0;
nodelist.alloc = 256;
nodelist.list = MVM_malloc(nodelist.alloc * sizeof(MVMProfileCallNode *));

Expand Down

0 comments on commit 81f9ccd

Please sign in to comment.