4 ms·

I doubt this library is useful. Some of the provided functions such as Clone(), Clear(), Copy() and Equal() are substituted by simple and readable three-line lo

I doubt this library is useful. Some of the provided functions such as Clone(), Clear(), Copy() and Equal() are substituted by simple and readable three-line loops. Others are plain harmful. For example, Keys() and Values() may lead to huge memory allocations when working over big maps, while DeleteFunc() worsens code readability and maintainability comparing to a simple 3-line loop. Additionally, it may slow down the resulting code.

oauea 4y ago
Useful enough to be included in the standard library. https://github.com/golang/go/issues/57436#issuecomment-1412566002 https://github.com/golang/go/issues/57436#issuecomment-14125...
- valyala 4y ago
  That would be a mistake :(
  - oauea 4y ago
    FUD
jen20 4y ago
> I doubt this library is useful It is. > Additionally, it may slow down the resulting code. FUD.
- valyala 4y ago
  Agreed, it's my fault that I didn't explain why maps.DeleteFunc() can be slower than a simple loop with "if" statement inside. The reasons for the slowdown are: - Go compiler may fail to inline the callback passed to the maps.DeleteFunc(). This will result into an additional overhead for callback calls per each item in the map. - Go compiler may move some variables inside the callback from stack to a heap. This will result in an additional memory allocations comparing to a simple loop, leading to an additional load on garbage collector.
0x62c1b43e 4y ago
By that logic we should remove bytes.Clone, strings.Split, bytes.Equal, strings.TrimLeftFunc, etc. from the standard library.
- valyala 4y ago
  The bytes.Clone() uses non-trivial trick under the hood - `append([]byte{}, b...)`. Compare it to a trivial loop behind maps.Clone(). The strings.Split() implementation is non-trivial because of performance optimizations. The bytes.Equal() is actually written in highly tuned and optimized assembly in order to achieve high performance for inputs of various lengths. Now compare this to trivial implementations behind generic-based functions for maps. And do not forget that these implementations may hurt performance because of excess memory allocations in Keys() and Values() functions or because the compiler may fail inlining the callback passed to DeleteFunc().
  - 0x62c1b43e 4y ago
    Those are good points about some of them being nontrivial. Though part of why they’re optimized and in the stdlib in the first place is because they’re such common patterns. So without them people would end up writing trivial, unperformant, custom versions. So now that more routines can be moved into the stdlib, they can benefit from optimization later. (I’m not sure how much the maps routines specifically can be optimized, but stdlib routines routines can generally be more aggressive with unsafe or asm or being coupled to the runtime and its quirks, like bytes.Clone, strings.Builder, etc.) And there are still plenty of ubiquitous patterns that have been worth including in the stdlib even if they’re usually just simple loops that aren’t very optimizable. Like strings.Index is an easy loop to write, but it comes up so often. Or strings.Cut is basically just an if-statement. But it makes code clearer about its intentions; and optimizations to these down the road benefit everyone. It’s also true that maps.Keys and maps.Values allocate slices, and that you could avoid this with a loop, but strings.Split, bytes.Split, regexp.FindAll, os.ReadDir return slices and are still worthwhile as opposed to specialized iterators for each one. As with any code, you’re conscious of memory allocations where it counts, and optimize as needed. In fact, now that generics make it possible, the Go team has discussed using iterators (https://github.com/golang/go/discussions/54245 https://github.com/golang/go/discussions/54245), which would benefit strings.Split even further in addition to all the other slice-returning functions. So generally you have three options for those slice-returning functions: - Custom inline loop for some of them. More verbose, will probably be naive and not benefit from stdlib optimizations. - Return a slice and iterate over it with a for loop. Creates allocations that could probably be avoided. - Create a customized iterator for that type. Unfortunately, you can’t really use an ordinary for loop, and extra custom iterators for each type. - Use generic iterators to benefit from the optimized functions and also avoid allocation overhead. So part of the motivation is that now with generics there’s a variety of further optimizations available even to old functions like strings.Split and regexp.FindAll, in addition to opening up common patterns and optimizations for maps/slices/etc. to be included in the stdlib.

oauea 4y ago

Useful enough to be included in the standard library. https://github.com/golang/go/issues/57436#issuecomment-1412566002 https://github.com/golang/go/issues/57436#issuecomment-14125...

valyala 4y ago
That would be a mistake :(
- oauea 4y ago
  FUD

jen20 4y ago

> I doubt this library is useful It is. > Additionally, it may slow down the resulting code. FUD.

valyala 4y ago
Agreed, it's my fault that I didn't explain why maps.DeleteFunc() can be slower than a simple loop with "if" statement inside. The reasons for the slowdown are: - Go compiler may fail to inline the callback passed to the maps.DeleteFunc(). This will result into an additional overhead for callback calls per each item in the map. - Go compiler may move some variables inside the callback from stack to a heap. This will result in an additional memory allocations comparing to a simple loop, leading to an additional load on garbage collector.

0x62c1b43e 4y ago

By that logic we should remove bytes.Clone, strings.Split, bytes.Equal, strings.TrimLeftFunc, etc. from the standard library.

valyala 4y ago
The bytes.Clone() uses non-trivial trick under the hood - `append([]byte{}, b...)`. Compare it to a trivial loop behind maps.Clone(). The strings.Split() implementation is non-trivial because of performance optimizations. The bytes.Equal() is actually written in highly tuned and optimized assembly in order to achieve high performance for inputs of various lengths. Now compare this to trivial implementations behind generic-based functions for maps. And do not forget that these implementations may hurt performance because of excess memory allocations in Keys() and Values() functions or because the compiler may fail inlining the callback passed to DeleteFunc().
- 0x62c1b43e 4y ago
  Those are good points about some of them being nontrivial. Though part of why they’re optimized and in the stdlib in the first place is because they’re such common patterns. So without them people would end up writing trivial, unperformant, custom versions. So now that more routines can be moved into the stdlib, they can benefit from optimization later. (I’m not sure how much the maps routines specifically can be optimized, but stdlib routines routines can generally be more aggressive with unsafe or asm or being coupled to the runtime and its quirks, like bytes.Clone, strings.Builder, etc.) And there are still plenty of ubiquitous patterns that have been worth including in the stdlib even if they’re usually just simple loops that aren’t very optimizable. Like strings.Index is an easy loop to write, but it comes up so often. Or strings.Cut is basically just an if-statement. But it makes code clearer about its intentions; and optimizations to these down the road benefit everyone. It’s also true that maps.Keys and maps.Values allocate slices, and that you could avoid this with a loop, but strings.Split, bytes.Split, regexp.FindAll, os.ReadDir return slices and are still worthwhile as opposed to specialized iterators for each one. As with any code, you’re conscious of memory allocations where it counts, and optimize as needed. In fact, now that generics make it possible, the Go team has discussed using iterators (https://github.com/golang/go/discussions/54245 https://github.com/golang/go/discussions/54245), which would benefit strings.Split even further in addition to all the other slice-returning functions. So generally you have three options for those slice-returning functions: - Custom inline loop for some of them. More verbose, will probably be naive and not benefit from stdlib optimizations. - Return a slice and iterate over it with a for loop. Creates allocations that could probably be avoided. - Create a customized iterator for that type. Unfortunately, you can’t really use an ordinary for loop, and extra custom iterators for each type. - Use generic iterators to benefit from the optimized functions and also avoid allocation overhead. So part of the motivation is that now with generics there’s a variety of further optimizations available even to old functions like strings.Split and regexp.FindAll, in addition to opening up common patterns and optimizations for maps/slices/etc. to be included in the stdlib.