alamb commented on code in PR #10033:
URL:
https://github.com/apache/arrow-datafusion/pull/10033#discussion_r1569167468
##########
datafusion/core/src/dataframe/mod.rs:
##########
@@ -156,7 +156,7 @@ impl Default for DataFrameWriteOptions {
/// ```
#[derive(Debug, Clone)]
pub struct DataFrame {
- session_state: SessionState,
+ session_state: Box<SessionState>,
Review Comment:
Maybe we can add a rationale here too so we don't forget why we did this.
Perhaps something like
```suggestion
// Box the (large) SessionState to reduce the size of DataFrame on the
stack
session_state: Box<SessionState>,
```
##########
Cargo.toml:
##########
@@ -126,3 +126,6 @@ opt-level = 3
overflow-checks = false
panic = 'unwind'
rpath = false
+
+[workspace.lints.clippy]
+large_futures = "warn"
Review Comment:
I think it would help to leave some rationale here about why we have enabled
this lint. Maybe something like
```suggestion
[workspace.lints.clippy]
# large stack frames cause overflows on debug builds, so try and avoid them
large_futures = "warn"
```
##########
clippy.toml:
##########
@@ -6,3 +6,7 @@ disallowed-methods = [
disallowed-types = [
{ path = "std::time::Instant", reason = "Use
`datafusion_common::instant::Instant` instead for WASM compatibility" },
]
+
+# Lowering the threshold to help prevent stack overflows (default is 16384)
+# See: https://rust-lang.github.io/rust-clippy/master/index.html#/large_futures
Review Comment:
Thank you for leaving the comment here
--
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]
