I have the following code:
import
tables
type
SomeEnum = enum
PARENT, CHILD
RefSomeOne = ref SomeOne
SomeOne = object
case kind: SomeEnum
of PARENT:
children: Table[int, ptr SomeOne]
of CHILD:
id: int
parent: RefSomeOne
proc `=destroy`(x: var SomeOne) =
case x.kind:
of PARENT:
echo "destroy parent"
for key in x.children.keys:
x.children.del(key)
of CHILD:
echo "destroy child" & $x.id
if not x.parent.isNil():
if x.parent.children.hasKey(x.id):
x.parent.children.del(x.id)
proc getChild(parent: RefSomeOne): SomeOne =
let child_id = parent.children.len() + 1
result = SomeOne(kind: CHILD, id: child_id)
result.parent = parent
parent.children[child_id] = result.addr()
proc doChild(parent: RefSomeOne) =
var
child = getChild(parent)
discard child
proc doParent() =
var parent = RefSomeOne(kind: PARENT)
discard parent.getChild()
parent.doChild()
echo "len: ", parent.children.len()
proc doParentWithoutChild() =
var parent = RefSomeOne(kind: PARENT)
discard parent
echo "doParent:"
doParent()
echo "\ndoParentWithoutChild:"
doParentWithoutChild()
Output is:
doParent: destroy child2 len: 1 destroy child1 doParentWithoutChild: destroy parent
Why does doParent not call the finalizer of parent, while doParentWithoutChild does?
Ok, I'm still not exactly sure what you are wanting to achieve but I have an idea. I can help you understand why your example isn't working, but I would strongly advise using another method. There are simpler ways to do what you're wanting. :-)
The reason your parent isn't being free is that your overriding the default destroyer which properly de-initializes the child's reference to it's parent If you changed your destroyer to this, it'll free the parent properly:
proc `=destroy`(x: var SomeOne) =
...
of CHILD:
echo "destroy child" & $x.id
if not x.parent.isNil():
if x.parent.children.hasKey(x.id):
x.parent.children.del(x.id)
x.parent = nil # this part is important as it tells Nim to properly decrement the ref count to the parent
Needing to set x.parent = nil is why I mentioned that it's tricky to write your own destroyer= functions.
a) The item on the ref SomeOne would not work for my purpose, because I want the children used (but not returned) by nested procs to disappear from parent once the nested proc's task is done. This is why I want to invoke =destroy. The idea is, not to do it manually as I, and others, will forget doing it. Thus - as far as I can see - it is almost, but not quite, entirely unlike a non-solution.
It'd better to use the built-in tools if possible. It's less work for you. In this case, what you're wanting to accomplish can be done by using Table.pop. This will remove the child from the parent's Table and give it to your doChild. Then the child will be destroyed after the doChild proc is done. This works for both ref SomeOne and SomeOne versions.
b) The item on the object (got it as using SomeOne instead of the pointer) would do the job, but does not lead into a final call of the parent's finalizer either. ... And, well, calls =destroy quite frequent and, thus, neither seems too efficient nor esthetic.
When using value object (and not ref's) you will call destroy more often, but avoid memory allocations. Often it's faster to do lots of copies (and the associated destroys) on the stack than to do memory allocations, which can be quite slow since it can result in system calls and/or locks to acquire the new memory. Often it you can try both and see which is faster.
I tweaked your example using Table.pop using Table[int, RefSomeOne] and it works as I think you're wanting:
proc createChild(parent: RefSomeOne): int =
let child_id = parent.children.len() + 1
var child = RefSomeOne(kind: CHILD, id: child_id)
child.parent = parent
parent.children[child_id] = child
result = child_id
proc doChild(parent: RefSomeOne, child_id: int) =
var child: RefSomeOne
if parent.children.pop(child_id, child):
echo "child: ", $child.id
else:
echo "no child: ", child_id
proc doParent() =
var parent = RefSomeOne(kind: PARENT)
let child_id = parent.createChild()
parent.doChild(child_id)
echo "len: ", parent.children.len()
This separates out child creation and usage which makes it a bit easier to reason about. Also you only create one child. If you run this with nim r --gc:orc --d:traceArc finalizer_example.nim you get:
[Allocated] 0x7f3943737040 result: 0x7f3943737050
[Allocated] 0x7f3943737070 result: 0x7f3943737080
child: 1
destroy child1
[Freed] 0x7f3943737070
len: 0
destroy parent
[Freed] 0x7f3943737040
doParentWithoutChild:
[Allocated] 0x7f3943737040 result: 0x7f3943737050
destroy parent
[Freed] 0x7f3943737040
The two "[Allocated]" are from the -d:traceArc which tells you it's allocating the parent and then the child (only one child is created in my version). Both are free'ed as well. You can remove the destroy= and see that it works the same without them.
Thank you very much again.
Have revised & simplified my code in the meantime and included some of your advice - especially the one on the ref-count and the ptr.
With regards to your comments on item b, your doChild is actually the manual removal, I am trying to avoid using the destructor.
type
SomeEnum = enum
PARENT, CHILD
RefSomeOne = ref SomeOne
SomeOne = object
case kind: SomeEnum
of PARENT:
children: seq[SomeOne]
of CHILD:
id: int
parent: RefSomeOne
proc `=destroy`(x: var SomeOne) =
case x.kind:
of PARENT:
echo "destroy parent"
while x.children.len() > 0:
wasmoved(x.children[0])
x.children.delete(0)
of CHILD:
echo "destroy child" & $x.id
if not x.parent.isNil():
for pos in countup(x.parent.children.low, x.parent.children.high):
if x.parent.children[pos].id == x.id:
wasmoved(x.parent.children[pos])
x.parent.children.delete(pos)
break
x.parent = nil
proc getChild(parent: RefSomeOne): SomeOne =
let child_id = parent.children.len() + 1
result = SomeOne(kind: CHILD, id: child_id)
result.parent = parent
parent.children.add result
proc doParent() =
var
parent = RefSomeOne(kind: PARENT)
block:
discard parent.getChild()
doAssert parent.children.len() == 1
block:
discard parent.getChild()
doAssert parent.children.len() == 2
doAssert parent.children.len() == 1
doAssert parent.children.len() == 0
echo "parent still exists - 'destroy' not invoked: ", not parent.isNil()
proc doParentWithoutChild() =
var parent = RefSomeOne(kind: PARENT)
discard parent
echo "doParent:"
doParent()
echo "\ndoParentWithoutChild:"
doParentWithoutChild()
Output (with --gc:arc):
doParent: destroy child2 destroy parent destroy child1 destroy parent parent still exists - 'destroy' not invoked: true doParentWithoutChild: destroy parent
As you can see, =destroy doesn't get called at the end of doParent. Question is why?
It should actually be used as the finalizer for the parent of type RefSomeOne at the end of the proc, right?
Well ...
Here comes the code that works:
type
SomeEnum = enum
PARENT, CHILD
RefSomeOne = ref SomeOne
SomeOne = object
case kind: SomeEnum
of PARENT:
children: seq[SomeOne]
of CHILD:
id: int
parent: RefSomeOne
proc `=destroy`(x: var SomeOne) =
case x.kind:
of PARENT:
echo "destroy parent"
while x.children.len() > 0:
wasmoved(x.children[0])
x.children.delete(0)
of CHILD:
echo "destroy child" & $x.id
if not x.parent.isNil():
for pos in countup(x.parent.children.low, x.parent.children.high):
if x.parent.children[pos].id == x.id:
x.parent.children[pos].parent = nil
wasmoved(x.parent.children[pos])
x.parent.children.delete(pos)
break
x.parent = nil
proc getChild(parent: RefSomeOne): SomeOne =
let child_id = parent.children.len() + 1
result = SomeOne(kind: CHILD, id: child_id, parent: parent)
parent.children.add result
proc doParent() =
var
parent = RefSomeOne(kind: PARENT)
block:
discard parent.getChild()
doAssert parent.children.len() == 1
block:
discard parent.getChild()
doAssert parent.children.len() == 2
doAssert parent.children.len() == 1
doAssert parent.children.len() == 0
proc doParentWithoutChild() =
var parent = RefSomeOne(kind: PARENT)
discard parent
echo "doParent:"
doParent()
echo "\ndoParentWithoutChild:"
doParentWithoutChild()
Output:
doParent: destroy child2 destroy parent destroy child1 destroy parent parent is destroyed: destroy parent doParentWithoutChild: destroy parent
Thank you very much!
You’re welcome.
As you can see, =destroy doesn't get called at the end of doParent. Question is why?
Not sure, your mods seem to have broken it again. Probably missing another ref or you still have a cycle, etc. Maybe the children.destroy setup. You’re introducing cycles by the .parent field so you’ll also want to use GC:orc, not GC:arc. Your doParent is odd since it’s calling “parent destroy” twice but not at the end. Seems wrong.
Doing the manual “pop” isn’t the same as manually freeing the item. Also using SomeRef and returning the item probably isn’t doing what you want if you’re trying to “cache” items. You’ll end up with two different copies I think. That’s where a ref can make sense as it’d make one shared child.
However, if you want to create a bunch of children and free them once the parent is freed, just put them as seq[RefSomeOme] and use GC:orc with the cycle collector.
For the sake of completeness:
Your doParent is odd since it’s calling “parent destroy” twice but not at the end.
Had another look at this item. It's because I am using discard (& I shouldn't). Double call at the end is avoided by:
proc doParent() =
var
parent = RefSomeOne(kind: PARENT)
var child_1 = parent.getChild()
discard child_1
block:
var child_2 = parent.getChild()
discard child_2
doAssert parent.children.len() == 2
block:
var child_3 = parent.getChild()
discard child_3
doAssert parent.children.len() == 3
doAssert parent.children.len() == 2
doAssert parent.children.len() == 1
echo "parent is destroyed:"
Also using SomeRef and returning the item probably isn’t doing what you want if you’re trying to “cache” items.
Valid comment ... Considering the above even more ...
What in my eyes looks wrong is the fact that destroy is invoked for the parent whenever the x.parent.children.delete(pos) is invoked, although it is already nil. Tried what I could to inhibit with .nodestroy - no success (v 1.5.1 orc/arc with/without --experimental).
Perhaps I understand what might be occurring.. Many destroy's will include a nil check first thing. The compiler doesn't know at compile time if a given object is destroyed already, so anytime you do the .destroy or obj = nil it can invoke the destroyer. So while doing the semi-manual memory management can be done ... it's tricky!
Also, since a non-ref object lives on the stack, in theory you can call destroy= on it any number of times since you can't free the memory. Normally ARC will only call it once, but if you're overriding the destroy= then you have to put quite a bit of thought into manually calling the destroys, etc.
Personally I only use destroy'ers on ref types to free C structs that were manually allocated by C-libs. Even then I'd prefer not to do it.
Valid comment ... Considering the above even more ...
Thanks I've actually learned a bit here too. It's a good way to dive in.. but probably better to use built-in tools and let the compiler deal with it as @araq mentioned. ;-)
If you're wanting to do "high-performance" memory (like for a game engine) then there's good options like weave. Though I'd recommend the simpler seq[RefSomeObj] until you have an explicit need. The Nim compiler and std-lib do pretty well. Perhaps you might sprinkle in some move annotations or profile the ref-vs-value types.
Happy Nim'ing!
Thanks I've actually learned a bit here too.
Well, my learning curve was somewhat steeper, thank you again. And if I understood the background sufficiently well, I might have found a reasonable solution for my issue that involves the lessons learned.
For me, it looks sufficiently foolproof, although it involves (a pretty basic) =destroy. It keeps the cached children as ref objects (yes, far better) and, well, seems to do the trick required ...
import
tables
type
SomeEnum = enum
PARENT, CHILD
RefSomeOne = ref SomeOne
SomeOne = object
case kind: SomeEnum
of PARENT:
children: Table[int, RefSomeOne]
of CHILD:
id: int
parent: RefSomeOne
ChildErasor = object
id: int
parent: RefSomeOne
proc `=destroy`(x: var ChildErasor) =
# The foolproof part:
if not x.parent.isNil():
var child: RefSomeOne
if x.parent.children.pop(x.id, child):
echo "erase child" & $x.id
# ------------------------------------------------
# the really important piece (NEVER, NEVER, EVER FORGET!):
x.parent = nil
proc getChild(parent: RefSomeOne): tuple[child: RefSomeOne, erasor: ChildErasor] =
let
child_id = parent.children.len() + 1
child = RefSomeOne(kind: CHILD, id: child_id, parent: parent)
erasor = ChildErasor(id: child_id, parent: parent)
parent.children[child_id] = child
result.child = child
result.erasor = erasor
var
parent = RefSomeOne(kind: PARENT)
child_1 = parent.getChild()
discard child_1
block:
var child_2 = parent.getChild()
discard child_2
doAssert parent.children.len() == 2
block:
var child_3 = parent.getChild()
discard child_3
doAssert parent.children.len() == 3
doAssert parent.children.len() == 2
doAssert parent.children.len() == 1
Output:
[Allocated] 0000000000670040 result: 0000000000670050 [Allocated] 0000000000670070 result: 0000000000670080 [Allocated] 00000000006700A0 result: 00000000006700B0 [Allocated] 00000000006700D0 result: 00000000006700E0 erase child3 [Freed] 00000000006700D0 erase child2 [Freed] 00000000006700A0 erase child1 [Freed] 0000000000670070 [Freed] 0000000000670040
Have also thought about sending Andreas lots of chocolates, sweets and flowers for him to invoke some at_destroy(x: var T) (= default, if not overloaded) at the very beginning of nim's standard =destroy (no clue how costly/possible): It's not too uncommon that something is to be done by standard before destroying object instances (cleanup, tasks etc.). Although it might add some flexibility, avoid repetitive code and be even more foolproof, I think I can live with the above for the moment. Sorry, Andreas, for the chocolate & flowers.
Sorry, Andreas, for the chocolate & flowers.
It's fine, I'm growing too fat anyway.