205

How can one remove selected keys from a map? Is it safe to combine delete() with range, as in the code below?

package main

import "fmt"

type Info struct {
    value string
}

func main() {
    table := make(map[string]*Info)

    for i := 0; i < 10; i++ {
        str := fmt.Sprintf("%v", i)
        table[str] = &Info{str}
    }

    for key, value := range table {
        fmt.Printf("deleting %v=>%v\n", key, value.value)
        delete(table, key)
    }
}

https://play.golang.org/p/u1vufvEjSw

Everton
  • 12,589
  • 9
  • 47
  • 59

4 Answers4

271

This is safe! You can also find a similar sample in Effective Go:

for key := range m {
    if key.expired() {
        delete(m, key)
    }
}

And the language specification:

The iteration order over maps is not specified and is not guaranteed to be the same from one iteration to the next. If map entries that have not yet been reached are removed during iteration, the corresponding iteration values will not be produced. If map entries are created during iteration, that entry may be produced during the iteration or may be skipped. The choice may vary for each entry created and from one iteration to the next. If the map is nil, the number of iterations is 0.

Dave C
  • 7,729
  • 4
  • 49
  • 65
Sebastian
  • 16,813
  • 4
  • 49
  • 56
  • key.expired undefined (type string has no field or method expired) –  Feb 15 '16 at 02:28
  • 7
    @kristen -- in the example described above, the key should not be a string but rather some custom type that implements the `func (a T) expired() bool` interface. For purposes of this example, you could try: `m := make(map[int]int)` `/* populate m here somehow */` `for key := range (m) {` `if key % 2 == 0 { /* this is just some condition, such as calling expired */` `delete(m, key);` `}` `}` – abanana Jan 19 '17 at 15:40
  • Very confusing. – g10guang Jul 25 '19 at 09:02
  • "If map entries are created during iteration, that entry may be produced during the iteration or may be skipped." -> while the deletion might be safe, this sentence makes modifying the map during iteration a code smell, IMO – gokul_uf Oct 20 '22 at 23:11
216

Sebastian's answer is accurate, but I wanted to know why it was safe, so I did some digging into the Map source code. It looks like on a call to delete(k, v), it basically just sets a flag (as well as changing the count value) instead of actually deleting the value:

b->tophash[i] = Empty;

(Empty is a constant for the value 0)

What the map appears to actually be doing is allocating a set number of buckets depending on the size of the map, which grows as you perform inserts at the rate of 2^B (from this source code):

byte    *buckets;     // array of 2^B Buckets. may be nil if count==0.

So there are almost always more buckets allocated than you're using, and when you do a range over the map, it checks that tophash value of each bucket in that 2^B to see if it can skip over it.

To summarize, the delete within a range is safe because the data is technically still there, but when it checks the tophash it sees that it can just skip over it and not include it in whatever range operation you're performing. The source code even includes a TODO:

 // TODO: consolidate buckets if they are mostly empty
 // can only consolidate if there are no live iterators at this size.

This explains why using the delete(k,v) function doesn't actually free up memory, just removes it from the list of buckets you're allowed to access. If you want to free up the actual memory you'll need to make the entire map unreachable so that garbage collection will step in. You can do this using a line like

map = nil
maksadbek
  • 1,508
  • 2
  • 15
  • 28
Verran
  • 3,942
  • 2
  • 14
  • 22
  • 2
    So it sounds like you're saying it's safe to delete any arbitrary value from the map,not just the 'current' one, correct? And when it comes time to evaluate a hash which I have previously arbitrarily deleted, it will safely skip over it? – Jonathan Hall Feb 27 '15 at 22:18
  • @Flimzy That is correct, as you can see from this playground http://play.golang.org/p/FwbsghzrsO . Note that if the index you delete is the first one in the range, it will still show that one since its already written to k,v but if you set the index to any besides the first one that range finds it will only display two key/value pairs instead of three and not panic. – Verran Mar 02 '15 at 17:38
  • 3
    Is the "doesn't actually free up memory" still relevant? I tried to find in the source that comment but cannot find it. – Tony Jan 19 '16 at 01:23
  • 19
    **Important note:** remember that this is just the *current implementation*, and it may change in future, so you must not rely on any additional properties it may appear to "support". The ***only guarantees you have are those provided by the specification, [as described in Sebastian's answer](http://stackoverflow.com/a/23230406/98528).*** (That said, exploring and explaining the Go internals is sure interesting, educating and generally awesome!) – akavel Aug 25 '16 at 13:03
10

I was wondering if a memory leak could happen. So I wrote a test program:

package main

import (
    log "github.com/Sirupsen/logrus"
    "os/signal"
    "os"
    "math/rand"
    "time"
)

func main() {
    log.Info("=== START ===")
    defer func() { log.Info("=== DONE ===") }()

    go func() {
        m := make(map[string]string)
        for {
            k := GenerateRandStr(1024)
            m[k] = GenerateRandStr(1024*1024)

            for k2, _ := range m {
                delete(m, k2)
                break
            }
        }
    }()

    osSignals := make(chan os.Signal, 1)
    signal.Notify(osSignals, os.Interrupt)
    for {
        select {
        case <-osSignals:
            log.Info("Recieved ^C command. Exit")
            return
        }
    }
}

func GenerateRandStr(n int) string {
    rand.Seed(time.Now().UnixNano())
    const letterBytes = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"
    b := make([]byte, n)
    for i := range b {
        b[i] = letterBytes[rand.Int63() % int64(len(letterBytes))]
    }
    return string(b)
}

Looks like GC do frees the memory. So it's okay.

vitvlkv
  • 782
  • 7
  • 13
5

In short, yes. See previous answers.

And also this, from here:

ianlancetaylor commented on Feb 18, 2015
I think the key to understanding this is to realize that while executing the body of a for/range statement, there is no current iteration. There is a set of values that have been seen, and a set of values that have not been seen. While executing the body, one of the key/value pairs that has been seen--the most recent pair--was assigned to the variable(s) of the range statement. There is nothing special about that key/value pair, it's just one of the ones that has already been seen during the iteration.

The question he's answering is about modifying map elements in place during a range operation, which is why he mentions the "current iteration". But it's also relevant here: you can delete keys during a range, and that just means that you won't see them later on in the range (and if you already saw them, that's okay).

Larry Clapp
  • 101
  • 1
  • 5