isAlive() too conservative -- does it cause leaks?

Hi Simon, Currently isAlive considers all static closures as being alive. The code: // ignore static closures // // ToDo: This means we never look through IND_STATIC, which means // isRetainer needs to handle the IND_STATIC case rather than // raising an error. // // ToDo: for static closures, check the static link field. // Problem here is that we sometimes don't set the link field, eg. // for static closures with an empty SRT or CONSTR_NOCAFs. // if (!HEAP_ALLOCED_GC(q)) { return p; } I'd expect this to cause leaks when e.g. key of a WEAK is a static object. Is this not the case? I think this is easy to fix but I may be missing something and wanted to ask before investing into it. The idea: - Evacuate all static objects in evacuate() (including the ones with no SRTs) (assuming all static objects have a STATIC_FIELD, is this really the case?) - In isAlive() check if (STATIC_FIELD & static_flag) != 0. If it is then the object is alive. Am I missing anything? Thanks, Ömer

On 19 July 2018 at 11:09, Ömer Sinan Ağacan
Hi Simon,
Currently isAlive considers all static closures as being alive. The code:
// ignore static closures // // ToDo: This means we never look through IND_STATIC, which means // isRetainer needs to handle the IND_STATIC case rather than // raising an error. // // ToDo: for static closures, check the static link field. // Problem here is that we sometimes don't set the link field, eg. // for static closures with an empty SRT or CONSTR_NOCAFs. // if (!HEAP_ALLOCED_GC(q)) { return p; }
I'd expect this to cause leaks when e.g. key of a WEAK is a static object. Is this not the case?
Correct, I believe weak pointers to static objects don't work (not sure if there's a ticket for this, but if not there should be). I think this is easy to fix but I may be missing something
and wanted to ask before investing into it. The idea:
- Evacuate all static objects in evacuate() (including the ones with no SRTs) (assuming all static objects have a STATIC_FIELD, is this really the case?)
This would be expensive. We deliberately don't touch the static objects in a minor GC because it adds potentially tens of ms to the GC time, and the optimisation to avoid evacuating the static objects with no SRTs is an important one. Cheers Simon - In isAlive() check if (STATIC_FIELD & static_flag) != 0. If it is then the
object is alive.
Am I missing anything?
Thanks,
Ömer

I created https://ghc.haskell.org/trac/ghc/ticket/15417 for this.
Ömer
Simon Marlow
On 19 July 2018 at 11:09, Ömer Sinan Ağacan
wrote: Hi Simon,
Currently isAlive considers all static closures as being alive. The code:
// ignore static closures // // ToDo: This means we never look through IND_STATIC, which means // isRetainer needs to handle the IND_STATIC case rather than // raising an error. // // ToDo: for static closures, check the static link field. // Problem here is that we sometimes don't set the link field, eg. // for static closures with an empty SRT or CONSTR_NOCAFs. // if (!HEAP_ALLOCED_GC(q)) { return p; }
I'd expect this to cause leaks when e.g. key of a WEAK is a static object. Is this not the case?
Correct, I believe weak pointers to static objects don't work (not sure if there's a ticket for this, but if not there should be).
I think this is easy to fix but I may be missing something and wanted to ask before investing into it. The idea:
- Evacuate all static objects in evacuate() (including the ones with no SRTs) (assuming all static objects have a STATIC_FIELD, is this really the case?)
This would be expensive. We deliberately don't touch the static objects in a minor GC because it adds potentially tens of ms to the GC time, and the optimisation to avoid evacuating the static objects with no SRTs is an important one.
Cheers Simon
- In isAlive() check if (STATIC_FIELD & static_flag) != 0. If it is then the object is alive.
Am I missing anything?
Thanks,
Ömer
participants (2)
-
Simon Marlow
-
Ömer Sinan Ağacan