all: assign zero after resize in implementations of heap.Interface #26296
Conversation
estensen
requested review from
fjl,
zsfelfoldi,
karalabe,
holiman and
rjl493456442
as code owners
| old[n-1] = nil | ||
| *s = old[0 : n-1] |
There was a problem hiding this comment.
Interesting idea -- but I think this should not be needed. Instead of nilling the item, we resize the slice.
Taken to the extreme, imagine we have a slice with 100 pointers, and then do myslice = myslice[:0] -- that should free up all the pointers for the gc, without us explicitly nilling them. It's the same case here.
There was a problem hiding this comment.
I don't think resizing would help since the same underlying array is referenced.
This is the example for a priority queue from the standard lib:
func (pq *PriorityQueue) Pop() any {
old := *pq
n := len(old)
item := old[n-1]
old[n-1] = nil // avoid memory leak
item.index = -1 // for safety
*pq = old[0 : n-1]
return item
}
| @@ -70,6 +70,7 @@ func (h *expHeap) Pop() interface{} { | |||
| old := *h | |||
| n := len(old) | |||
| x := old[n-1] | |||
| old[n-1] = expItem{} | |||
There was a problem hiding this comment.
IMO this is moot, and will be forgotten by the next line
| @@ -45,6 +45,7 @@ func (h *nonceHeap) Pop() interface{} { | |||
| old := *h | |||
| n := len(old) | |||
| x := old[n-1] | |||
| old[n-1] = 0 | |||
There was a problem hiding this comment.
Setting a uint64 to 0 does not make any difference to the gc
There was a problem hiding this comment.
Slice tricks from the Go wiki states that you should set the item that is deleted to the zero value of T.
fjl
changed the title
…thereum#26296) This changes the Pop method to assign the zero value before reducing slice size. Doing so ensures the backing array does not reference removed item values.
Set the deleted item to its zero val so it can be garbage collected.