Add generic setters to void casting values to any #276
Conversation
Casting to any can be a significant overhead for table functions, as it usually requires an allocation.
|
Update: Removed all casting logic other than the new set interface |
There was a problem hiding this comment.
Hi @JAicewizard, thanks for the PR; this is great!
I've left some nits. 😄
Did you happen to run the benchmark func BenchmarkTypes(b *testing.B) in types_test.go? I'd be curious about the improvements.
I also saw that you added code to auto-cast between some types, e.g., in setBool. I am unsure if we should add that or whether it should happen on the user side. Also because we currently only support it for some types... 🤔
I did this just now, but most of that benchmark is spend making queries and not actually setting the values using the appender. But I checked using pprof and the time spend setting the values went from 11% to 8%. Not sure how reliable that is. Most of the time is spend in duckdb, so we cannot really improve that. |
JAicewizard
force-pushed
the
generic_vector
branch
from
cb43f65 to
8c4f21b
Compare
|
Alright, thanks a lot for your work on this! I'll merge it once the tests pass. |
|
I wrote some benchmarks, and they showed a 20% in time spend appending. I also uncovered some crashes however, which should be investigated. I will open a PR with the benchmark, but I don't have a fix for the crashes yet |
Casting to any can be a significant overhead for table functions, as it usually requires an allocation.
In addition, the new functions also handle all possible castings that we support, so I think that we can also remove
tryCastcompletely saving 200 linex, and I suspect we can even remove thesetFnfield, saving even more. Maybe someone can look at that to see if that sounds reasonable?