[SOLVED]garbage collector + stack of startup task

C programming, build, interpreter/VM.
Target audience: MicroPython Developers.
BramPeeters
Posts: 54
Joined: Wed Jan 31, 2018 3:10 pm

Re: garbage collector + stack of startup task

Post by BramPeeters » Wed Sep 12, 2018 10:40 pm

. I enjoy this.
Lol :p

jickster
Posts: 629
Joined: Thu Sep 07, 2017 8:57 pm

Re: garbage collector + stack of startup task

Post by jickster » Wed Sep 12, 2018 10:41 pm

You can turn on full debugging of gc to print out status of every allocation and freeing.

Then compare allocations and freeings.


Sent from my iPhone using Tapatalk Pro

BramPeeters
Posts: 54
Joined: Wed Jan 31, 2018 3:10 pm

Re: garbage collector + stack of startup task

Post by BramPeeters » Wed Sep 12, 2018 10:57 pm

I tried that (enabling verbose mode in mpportconfig I assume you mean) .
That is when i saw that if i moved the stack onto the heap there were only +- 5 sweep (=free) messages (and it kept working) versus +-20 sweeps (and it crashed after suspend/resuming) if it was still in static memory during the first call of gc.collect() (if i call it every 100ms).

So that triggered my whole 'the contents of the static stack seem to be ignored' idea and when i looked at the code i thought i found the explanation ('if you are a hammer everything looks like a nail' - wise ).

So I will have to look again into why these sweep results are so different between the 2 scenarios, but it is a good starting point. And in the meantime I've learned a lot more about the GC :) . And optimized the external memory parameters so that it is now much faster which was needed with my initial stack located on the heap in external sram:).

jickster
Posts: 629
Joined: Thu Sep 07, 2017 8:57 pm

Re: garbage collector + stack of startup task

Post by jickster » Thu Sep 13, 2018 3:29 pm

BramPeeters wrote:
Wed Sep 12, 2018 10:57 pm
I tried that (enabling verbose mode in mpportconfig I assume you mean) .
That is when i saw that if i moved the stack onto the heap there were only +- 5 sweep (=free) messages (and it kept working) versus +-20 sweeps (and it crashed after suspend/resuming) if it was still in static memory during the first call of gc.collect() (if i call it every 100ms).

So that triggered my whole 'the contents of the static stack seem to be ignored' idea and when i looked at the code i thought i found the explanation ('if you are a hammer everything looks like a nail' - wise ).

So I will have to look again into why these sweep results are so different between the 2 scenarios, but it is a good starting point. And in the meantime I've learned a lot more about the GC :) . And optimized the external memory parameters so that it is now much faster which was needed with my initial stack located on the heap in external sram:).
I have an idea how to help you debug your gc issues

https://github.com/micropython/micropython/issues/4135

User avatar
dhylands
Posts: 3821
Joined: Mon Jan 06, 2014 6:08 pm
Location: Peachland, BC, Canada
Contact:

Re: garbage collector + stack of startup task

Post by dhylands » Thu Sep 13, 2018 4:29 pm

If the stack is located in the heap, then the entire stack area will be scanned, including the currently unused portion of the stack, and its quite possible that old stale pointers may be holding onto memory.

When the stack is scanned "as a stack", then it will only look from the current stack pointer thru to the top of the stack (so it only scans the currently used portion of the stack).

I haven't looked into how the stacks are scanned when threading is being used, but I would have thought it would need to know the stack pointer for each thread's stack and only scan the active portion of the stack.

jickster
Posts: 629
Joined: Thu Sep 07, 2017 8:57 pm

Re: garbage collector + stack of startup task

Post by jickster » Thu Sep 13, 2018 9:44 pm

BramPeeters wrote:
Wed Sep 12, 2018 10:57 pm

So I will have to look again into why these sweep results are so different between the 2 scenarios, but it is a good starting point. And in the meantime I've learned a lot more about the GC :) .
Post code.

jickster
Posts: 629
Joined: Thu Sep 07, 2017 8:57 pm

Re: garbage collector + stack of startup task

Post by jickster » Thu Sep 13, 2018 9:56 pm

dhylands wrote:
Thu Sep 13, 2018 4:29 pm
If the stack is located in the heap, then the entire stack area will be scanned, including the currently unused portion of the stack
It's only "scanned" i.e. treated as a root pointer if you tell the gc it's a root-pointer area OR something else points into it.
dhylands wrote:
Thu Sep 13, 2018 4:29 pm
If the stack is located in the heap, then the entire stack area will be scanned, including the currently unused portion of the stack, and its quite possible that old stale pointers may be holding onto memory.

When the stack is scanned "as a stack", then it will only look from the current stack pointer thru to the top of the stack (so it only scans the currently used portion of the stack).

I haven't looked into how the stacks are scanned when threading is being used, but I would have thought it would need to know the stack pointer for each thread's stack and only scan the active portion of the stack.

Code: Select all

void mp_thread_gc_others(void) {
    mp_thread_mutex_lock(&thread_mutex, 1);
    for (thread_t *th = thread; th != NULL; th = th->next) {
        gc_collect_root((void**)&th, 1);
        gc_collect_root(&th->arg, 1); // probably not needed
        if (th->id == xTaskGetCurrentTaskHandle()) {
            continue;
        }
        if (!th->ready) {
            continue;
        }
        gc_collect_root(th->stack, th->stack_len); // probably not needed
    }
    mp_thread_mutex_unlock(&thread_mutex);
}

User avatar
dhylands
Posts: 3821
Joined: Mon Jan 06, 2014 6:08 pm
Location: Peachland, BC, Canada
Contact:

Re: garbage collector + stack of startup task

Post by dhylands » Thu Sep 13, 2018 10:58 pm

jickster wrote:
Thu Sep 13, 2018 9:56 pm
dhylands wrote:
Thu Sep 13, 2018 4:29 pm
If the stack is located in the heap, then the entire stack area will be scanned, including the currently unused portion of the stack
It's only "scanned" i.e. treated as a root pointer if you tell the gc it's a root-pointer area OR something else points into it.
Well if your stack is allocated on the heap and you don't have a root-pointer (or some other object) pointing to it, then it will be released and your stack area will get used for regular allocations which will definitely crash things.

So my assumption was that if the stack is allocated on the heap, then you better have something pointing to it or you're going to be in a world of hurt when it gets used for something else :)

jickster
Posts: 629
Joined: Thu Sep 07, 2017 8:57 pm

Re: garbage collector + stack of startup task

Post by jickster » Thu Sep 13, 2018 10:59 pm

dhylands wrote:
jickster wrote:
Thu Sep 13, 2018 9:56 pm
dhylands wrote:
Thu Sep 13, 2018 4:29 pm
If the stack is located in the heap, then the entire stack area will be scanned, including the currently unused portion of the stack
It's only "scanned" i.e. treated as a root pointer if you tell the gc it's a root-pointer area OR something else points into it.
Well if your stack is allocated on the heap and you don't have a root-pointer (or some other object) pointing to it, then it will be released and your stack area will get used for regular allocations which will definitely crash things.

So my assumption was that if the stack is allocated on the heap, then you better have something pointing to it or you're going to be in a world of hurt when it gets used for something else :)
The feature I proposed would help debugging these type of issues


Sent from my iPhone using Tapatalk Pro

BramPeeters
Posts: 54
Joined: Wed Jan 31, 2018 3:10 pm

Re: garbage collector + stack of startup task

Post by BramPeeters » Mon Sep 17, 2018 1:30 pm

Small update, I located the issue, it was due to a bug in my porting :( .

Details:
In

Code: Select all

void mp_thread_init(void)
{
	mp_thread_mutex_init(&thread_mutex);
	mp_thread_set_state(&mp_state_ctx.thread);
	// create first entry in linked list of all threads
	thread = &thread_entry0;
	thread->id = xTaskGetCurrentTaskHandle();
	thread->ready = 1;
	thread->arg = NULL;
	thread->stack = mpTaskStack;
	thread->stack_len = MICROPY_TASK_STACK_LEN;
	thread->next = NULL;
}
the stack_len of the initial thread is assigned via MICROPY_TASK_STACK_LEN; , but in my port I used differently named defines for all my task stacks sizes (which had been configured to a size bigger than MICROPY_TASK_STACK_LEN;) and i had overlooked to change this one.

The end result was that when the stack was scanned in "gc_collect_root(th->stack, th->stack_len); // probably not needed", in

Code: Select all

void mp_thread_gc_others(void) {
    mp_thread_mutex_lock(&thread_mutex, 1);
    for (thread_t *th = thread; th != NULL; th = th->next) {
        gc_collect_root((void**)&th, 1);
        gc_collect_root(&th->arg, 1); // probably not needed
        if (th->id == xTaskGetCurrentTaskHandle()) {
            continue;
        }
        if (!th->ready) {
            continue;
        }
        gc_collect_root(th->stack, th->stack_len); // probably not needed
    }
    mp_thread_mutex_unlock(&thread_mutex);
}
that it stopped too soon and did not look at the tail of the stack where the most interesting stuff is located, hence some things got collected that shouldn't.

Sideremark:
The "//probably not needed" remark should 'probably' be removed because it is in fact really needed.
For the initial main task the thread_t struct is a static variable (cfr 'STATIC thread_t thread_entry0;'),
so attempting to pass it's contents via 'gc_collect_root((void**)&th, 1);' will not work as th is not located on the GC heap so it does not pass the VERIFY_PTR check.
You could try and fix this by forcing collect root to look at the contents by doing gc_collect_root((void*)th, sizeof(*th)/sizeof(void*) ), but even then when the pointer value at th->stack is presented to VERIFY_PTR it will be rejected because the stack is also not on the heap (and this time it will not first be dereferenced :) ).
So &th->arg and th->stack need to be explicitely passed to gc_collect_root in these circumstances.
This is only true if gc_collect is not called from that initial main task but from a different task, otherwise it is already taken care of via

Code: Select all

gc_collect_root((void**)sp, ((uint32_t)MP_STATE_THREAD(stack_top) - sp) / sizeof(uint32_t));
( Note: all this is specific for the cc3200 port & derived ports with freertos support )


Thanks for all the help and suggestions !

Post Reply