search

go-critic / go-critic

The most opinionated Go source code linter for code audit.
https://go-critic.com
MIT License
1.85k stars 116 forks source link

Optimize cases of clearing maps #832

Open breml opened 5 years ago

breml commented 5 years ago

Since merge of 110055 (Issue: go#20138, the clearing of maps (that is, delete all key/values from the map such that is empty) with a for ... range loop has been implemented in the compiler to emit optimized code. So go-critic could detect respective cases and suggest the necessary fix.

Before:

m = map[K]V{}

After:

for k := range m {
    delete(m, k)
}
quasilyte commented 5 years ago

I think the second form can take more CPU time. Need to check it out though. Never done any real measurements of this.

breml commented 5 years ago

Based on the benchamrks in 110055 I think it is the oposite. As said, the for ... range is detected by the compiler and replaced with highly optimized code to clear the map. The main advantage of this is, that there is no new allocation, which in fact has a positive effect on the GC.

Of course, we should also detect m = make(map[K]V).

breml commented 5 years ago

The difficult part maybe is to detect cases, where m is in fact not nil before the assignment of the new map.

quasilyte commented 5 years ago

I heard that there is something like memory/CPU time trade-off there anyway. For empty map literal there is only a (quite fast) allocation while clearing a huge map still requires to walk buckets and clearing them.

quasilyte commented 4 years ago

The difficult part maybe is to detect cases, where m is in fact not nil before the assignment of the new map.

I wonder whether there is a pass in go/analysis that can help with this. Probably something from nillness pass maybe. Although I haven't really looked at it. Maybe it doesn't return any "facts" after analysis for other passes. Anyway, if we can find a way to get that info for the checker, it would be a much easier task.