pixelherodev opened a new pull request, #556: URL: https://github.com/apache/arrow-go/pull/556
This primarily consists of reducing allocations. e.g. 03be6d3f adds a specific case to TypeEqual for ListView types, which avoids reflect.DeepEqual. l.elem.Equal does not allocate; reflect.DeepEqual does, so this reduces allocations when comparing a ListViewType. 8c7729fc changes from referencing flatbug internal types by reference to by-value, which avoids letting them escape to the heap. e3406582 switches from reflect.DeepEqual to slices.Equal when comparing the child IDs of a Union type, which is functionally equivalent for slices and, again, does not allocate. 9eba6d2c does similar for an arrowflight cookie test, replacing reflect.DeepEquals with maps.Equal; I did a grep for reflect.DeepEqual and noticed it. 7557d15c is probably the most objectionable; it replaces a *float64 with float64 directly for minSpaceSavings, since the current nil value has identical behavior to zero anyways. It's a tiny cleanup IMO, but doesn't really have any practical value. 2d3593f6 is a bit ugly; it inlines a copy of NewReaderFromMessageReader and a copy of NewMessageReader into NewReader - by implementing the two directly, it's able to avoid allocating two copies of the `config` structure, and to avoid looping over and invoking config functions twice. It's kinda ugly though, and the `// types: make(dictTypeMap)` which is copied over suggests that there's further room for improvement. I didn't drop the two APIs, as they are exposed to users, but IMO the performance win is probably worth it, and it can be cleaned up later if needed. 0273ffe8 is a small optimization to the allocator: the Go allocator banches on each allocation, but both branches are functionally equivalent, so I just merged the two. I doubt this would have a _major_ impact, since the cost of a branch is negligible compared to the cost of a heap allocation, but performance is often won with a lot of small impacts IMO :) Lastly, for now at least, f1a6a331 statically allocates 4 bytes per IPC messageReader (I did _not_ touch the ArrowFlight implementation), and uses it on every message read, rather than heap-allocating 4 bytes each time Message is called. There's more to be done, but this should be a much more practically sized chunk to review, and I can add more later :) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
