nat-openai commented on issue #5712: URL: https://github.com/apache/datafusion-comet/issues/5712#issuecomment-6155599197
https://github.com/apache/datafusion-comet/pull/6833 sped up the native kernel with the intent of mitigating point one in this issue. Regarding point 2, a rejection option is to introduce a new memory budget for sequences, which I experimented with in https://github.com/apache/datafusion-comet/pull/6819. The two other approaches I'm aware of (paraphrasing @andygrove's earlier comment) are 1. add a simple per-invocation limit on the output of a `sequence` call. There's an implementation of this in https://github.com/apache/datafusion-comet/pull/6939. In my understanding, a benefit of this is that it's pretty simple to understand (even moreso than #6819). On the other hand, it's a little unsatisfactory since valid queries that could fit in available memory can still be rejected whereas they would work before. 2. Use `try_grow`, which seems pretty elegant because it checks actual pool memory capacity can may trigger spilling downstream if necessary. In that respect it certainly seems better, but the implementation also appears to come with more tradeoffs (e.g. in the version I have, we lose constant folding and there's a ~6% slowdown with small sequences due to the additional accounting). I'm still very much a novice in the code base, but I think I have a slight preference for option (2), unless there are additional tradeoffs that I haven't yet identified. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
