feat(java): support session#5931
Conversation
Code Review: feat(java): support sessionP0 IssuesMemory leak in When pub fn handle_from_session(session: Arc<LanceSession>) -> jlong {
let boxed: Box<Arc<LanceSession>> = Box::new(session);
Box::into_raw(boxed) as jlong
}The Java side then wraps this in a new
Recommendation: Either:
P1 IssuesDefault cache size mismatch ( public static final long DEFAULT_INDEX_CACHE_SIZE_BYTES = 1L * 1024 * 1024 * 1024; // 1 GiB
public static final long DEFAULT_METADATA_CACHE_SIZE_BYTES = 256L * 1024 * 1024; // 256 MiBBut Rust defaults ( pub const DEFAULT_INDEX_CACHE_SIZE: usize = 6 * 1024 * 1024 * 1024; // 6 GiB
pub const DEFAULT_METADATA_CACHE_SIZE: usize = 1024 * 1024 * 1024; // 1 GiBAnd private long indexCacheSizeBytes = 6L * 1024 * 1024 * 1024; // 6 GiB
private long metadataCacheSizeBytes = 1024L * 1024 * 1024; // 1 GiBThe The test coverage is good, and the overall approach of exposing Session to Java is sound. The main concern is the potential memory leak from handles created via |
majin1102
left a comment
There was a problem hiding this comment.
LGTM, left one non-blocking comment
| if (session == null) { | ||
| long handle = nativeGetSessionHandle(); | ||
| if (handle != 0) { | ||
| session = Session.fromHandle(handle); |
There was a problem hiding this comment.
This might cause dangling references without write lock. That said, Rust's structures are shared and close() seems to work fine. Up to you.
There was a problem hiding this comment.
good point, I updated to just load it at build time
This allows Java to also pass in a Session shared across datasets similar to python and rust. Session can then be used for engine side caching implementation in Spark and Trino