Skip to content

Restore java serialize behavior - #3

Merged
HowardHuang1 merged 5 commits into
HowardHuang1:HH-Dataset-APIfrom
divyegala:java-serialize
Jul 30, 2026
Merged

Restore java serialize behavior#3
HowardHuang1 merged 5 commits into
HowardHuang1:HH-Dataset-APIfrom
divyegala:java-serialize

Conversation

@divyegala

Copy link
Copy Markdown

No description provided.

Comment on lines +654 to +655
MemorySegment datasetOutPtr = arena.allocate(cuvsDataset_t);
datasetOutPtr.set(cuvsDataset_t, 0, MemorySegment.NULL);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why we allocate dataset like this rather than call C API's factory?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only allocates the pointer-sized out-parameter slot required by Panama. The slot is initialized to NULL, and cuvsCagraDeserializeGraphAndDataset allocates the actual dataset and populates it. Calling cuvsDatasetCreate` first would pass a non-null handle and violate the deserializer contract.

* @param inputStream an instance of {@link InputStream}
* @return an instance of {@link IndexReference}
*/
private IndexReference deserialize(

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why we return IndexReference which is a pointer to the dataset? I thought outDataset was pre-allocated by the user and passed in by reference to be populated by within deserialize(). Is there a reason why we are moving away from this API contract and going back to the original where deserialize can internally allocate and hide memory allocations?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IndexReference is an internal Java construction result, not a dataset pointer or public return value. It carries the newly populated native index handle and optionally retains the dataset lifetime holder for the ordinary upstream load path. When outDataset is provided, that wrapper receives the single native dataset handle and IndexReference does not retain another owner.

@HowardHuang1 HowardHuang1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

this(inputStream, resources, null);
}

private CagraIndexImpl(

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this used for? Is this a java specific thing?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is Java-specific. The existing two-argument constructor preserves Builder.from(inputStream).build() behavior and delegates to the overload with no explicit output. The overload is used only when the caller asks to retain the deserialized dataset separately.

@HowardHuang1
HowardHuang1 merged commit 5581ffd into HowardHuang1:HH-Dataset-API Jul 30, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants