-
Notifications
You must be signed in to change notification settings - Fork 784
perf(session): support fragment session cache #7972
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -150,6 +150,82 @@ void testWriteFragmentWithSchemaOverride(@TempDir Path tempDir) throws Exception | |
| } | ||
| } | ||
|
|
||
| @Test | ||
| void testWriteFragmentWithSession(@TempDir Path tempDir) throws Exception { | ||
| String datasetPath = tempDir.resolve("fragment_with_session").toString(); | ||
| try (RootAllocator allocator = new RootAllocator(Long.MAX_VALUE)) { | ||
| TestUtils.SimpleTestDataset testDataset = | ||
| new TestUtils.SimpleTestDataset(allocator, datasetPath); | ||
| try (Dataset dataset = testDataset.createEmptyDataset(); | ||
| Session session = Session.builder().build()) { | ||
| // Write twice with the same session: APPEND-mode schema inference loads the | ||
| // manifest through the session's metadata cache instead of a fresh session | ||
| // per write. | ||
| for (int i = 0; i < 2; i++) { | ||
| try (VectorSchemaRoot root = VectorSchemaRoot.create(dataset.getSchema(), allocator)) { | ||
| root.allocateNew(); | ||
| ((VarCharVector) root.getVector("name")) | ||
| .setSafe(0, ("Person " + i).getBytes(StandardCharsets.UTF_8)); | ||
| ((IntVector) root.getVector("id")).setSafe(0, i); | ||
| root.setRowCount(1); | ||
|
|
||
| List<FragmentMetadata> fragments = | ||
| Fragment.write() | ||
| .datasetUri(datasetPath) | ||
| .allocator(allocator) | ||
| .data(root) | ||
| .mode(WriteParams.WriteMode.APPEND) | ||
| .session(session) | ||
| .execute(); | ||
| assertEquals(1, fragments.size()); | ||
| assertEquals(1, fragments.get(0).getPhysicalRows()); | ||
| } | ||
| } | ||
| // Schema inference populated the session's metadata cache. | ||
| assertTrue(session.sizeBytes() > 0); | ||
|
|
||
| // An explicit schema skips inference; the session is still accepted (used for | ||
| // the object store registry). | ||
| try (VectorSchemaRoot root = VectorSchemaRoot.create(dataset.getSchema(), allocator)) { | ||
| root.allocateNew(); | ||
| ((VarCharVector) root.getVector("name")) | ||
| .setSafe(0, "Person 2".getBytes(StandardCharsets.UTF_8)); | ||
| ((IntVector) root.getVector("id")).setSafe(0, 2); | ||
| root.setRowCount(1); | ||
|
|
||
| List<FragmentMetadata> fragments = | ||
| Fragment.write() | ||
| .datasetUri(datasetPath) | ||
| .allocator(allocator) | ||
| .data(root) | ||
| .schema(dataset.getLanceSchema()) | ||
| .mode(WriteParams.WriteMode.APPEND) | ||
| .session(session) | ||
| .execute(); | ||
| assertEquals(1, fragments.size()); | ||
|
Comment on lines
+172
to
+205
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win Cover the ArrowArrayStream session route. This test only exercises As per coding guidelines, “Every bugfix and feature must have corresponding tests.” 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| } | ||
| } | ||
|
|
||
| // A closed session is rejected instead of silently degrading to no session. | ||
| Session closedSession = Session.builder().build(); | ||
| closedSession.close(); | ||
| try (VectorSchemaRoot root = VectorSchemaRoot.create(testDataset.getSchema(), allocator)) { | ||
| root.allocateNew(); | ||
| root.setRowCount(0); | ||
| assertThrows( | ||
| IllegalArgumentException.class, | ||
| () -> | ||
| Fragment.write() | ||
| .datasetUri(datasetPath) | ||
| .allocator(allocator) | ||
| .data(root) | ||
| .mode(WriteParams.WriteMode.APPEND) | ||
| .session(closedSession) | ||
| .execute()); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| void commitWithoutVersion(@TempDir Path tempDir) { | ||
| String datasetPath = tempDir.resolve("commit_without_version").toString(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: lance-format/lance
Length of output: 13032
🏁 Script executed:
Repository: lance-format/lance
Length of output: 21824
🏁 Script executed:
Repository: lance-format/lance
Length of output: 31723
🌐 Web query:
Java JNI global reference ThreadLocalThreadState thread suspension garbage collection💡 Result:
The interaction between Java Native Interface (JNI) references, thread states, and garbage collection (GC) is governed by the Java Virtual Machine (JVM) mechanics that handle thread transitions between managed and native code [1][2][3]. JNI Global References Global references act as roots for the garbage collector, ensuring that the referenced objects remain reachable and are not prematurely reclaimed [4][5]. Because the GC may compact the heap and move objects, global references (and local references) provide a level of indirection [1][2]. Native code does not hold direct memory addresses to heap objects; instead, it interacts with these references, allowing the JVM to update the underlying object location during GC cycles without breaking the native code's access [1][2]. Thread Suspension and GC Safepoints JVMs typically use a safepoint mechanism to coordinate GC [1][2]. 1. Managed State: When a thread is executing Java bytecode, it is at a safepoint or can reach one, allowing the GC to suspend the thread and safely inspect its stack [1][3]. 2. Native State: When a thread transitions from Java to native code via JNI, it enters a state often referred to as _thread_in_native [1][2]. In this state, the thread is not executing Java bytecode and does not directly access the managed heap, meaning it can continue running concurrently with GC operations [1][2][3]. 3. Safepoint Barriers: The transition between native and managed states acts as a barrier [1][2]. When a native thread attempts to call back into the JVM or use a JNI function, it must check for safepoints [1][2][6]. If a GC is in progress or has requested a suspension, the thread will block until the GC operation completes [1][2][6]. JNI Critical Regions Certain JNI functions, such as GetPrimitiveArrayCritical, allow native code to access raw heap pointers directly [7][2]. These functions enter a "critical" state that often disables GC (or specific GC activities like compaction) for the duration of the region to prevent the memory from moving while the native code holds a direct pointer [7][2]. The JVM ensures that other threads attempting to trigger GC are aware of these active critical regions, often by using mechanisms like the GCLocker [7]. Recent developments in the OpenJDK (e.g., JDK 26 timeframe) have included refinements to how these regions interact with JVM Tool Interface (JVM TI) thread suspension, specifically aiming to defer suspension requests until after a critical region is exited to maintain system stability [8]. In summary, global references keep objects alive, native code execution is generally allowed to continue concurrently with GC, and thread suspension is enforced primarily at JNI boundaries where the thread must synchronize with the JVM's managed state [1][2][3].
Citations:
Hold a close-safe guard on
sessionHandleacross the JNI call.close()can run on another thread, and on most JVMs native code may be preempted after Java snapshotsnativeSessionHandle; the native side clones throughArcthat will have been released if Java zeroes the field. Keep a close/lifetime guard that is only dropped after the native Fragment write call returns, or move session ownership into that guard across bothcreateWithFfiArrayandcreateWithFfiStream.📍 Affects 2 files
java/src/main/java/org/lance/Fragment.java#L300-L304(this comment)java/src/main/java/org/lance/Fragment.java#L398-L402java/lance-jni/src/fragment.rs#L341-L343🤖 Prompt for AI Agents