[Java] Tidy up MemorySegments lifecycle#1069
[Java] Tidy up MemorySegments lifecycle#1069rapids-bot[bot] merged 15 commits intorapidsai:branch-25.08from
MemorySegments lifecycle#1069Conversation
ChrisHegarty
left a comment
ChrisHegarty
left a comment
There was a problem hiding this comment.
This is a good change. We should certainly be using more local arenas where possible, and cleaning up more eagerly. 👍
| var returnValue = cuvsRMMAlloc(resourceHandle, datasetMemorySegment, datasetBytes); | ||
| checkCuVSError(returnValue, "cuvsRMMAlloc"); | ||
|
|
||
| return datasetMemorySegment.get(C_POINTER, 0); |
cjnolet
added
improvement
…-memorysegment-scope
…-memorysegment-scope # Conflicts: # java/cuvs-java/src/main/java22/com/nvidia/cuvs/internal/BruteForceIndexImpl.java # java/cuvs-java/src/main/java22/com/nvidia/cuvs/internal/CagraIndexImpl.java # java/cuvs-java/src/main/java22/com/nvidia/cuvs/internal/HnswIndexImpl.java
|
FYI, https://termbin.com/ac7k3 here's a patch on top of this branch. It adds a synchronized() block around the search() call, and the test from #1082 (added in this patch) passes. |
For the record: we discussed this on slack and decided to keep the 2 separate, as they address different problems |
ldematte
changed the title
MemorySegments lifecycleMemorySegments lifecycle
|
/ok to test 250ca15 |
mythrocks
left a comment
mythrocks
left a comment
There was a problem hiding this comment.
LGTM! Waiting on CI now.
ldematte
changed the title
MemorySegments lifecycleMemorySegments lifecycle
…-memorysegment-scope
|
@mythrocks is anything blocking this from getting merged? Should I do something? |
|
/ok to test 9010448 |
Nothing. CI hadn't passed at the time. I'll check this in, once it does. |
|
The CI builds should be fixed once #1114 is resolved. |
|
/ok to test 109b481 |
|
Will prioritize #1034 first, to address the test failures. |
|
/ok to test 3ebc564 |
|
/ok to test d7badfd |
|
/merge |
github-project-automation
Bot
moved this from In Progress
to Done
in Unstructured Data Processing
github-project-automation
Bot
moved this from In Progress
to Done
in Elasticsearch + cuVS Team
|
This has been merged. Thank you for this contribution, @ldematte . |
|
Thank you for assisting through the process, @mythrocks! |
This PR tidies up the lifecycle of `MemorySegment`s by using specific confined `Arena`s where possible, and specific index-bound `Arena`s where the lifetime needs to be bound to the lifetime of an index. It addresses all remaining usages of `CuVSResources#arena` after rapidsai#1024 and rapidsai#1045; as such, we can remove `CuVSResources#arena` completely, and force native memory usages to be tracked and dealt with specifically. This would towards the goal of rapidsai#1037 Authors: - Lorenzo Dematté (https://github.com/ldematte) - MithunR (https://github.com/mythrocks) - Ben Frederickson (https://github.com/benfred) Approvers: - Chris Hegarty (https://github.com/ChrisHegarty) - MithunR (https://github.com/mythrocks) URL: rapidsai#1069
6 participants
This PR tidies up the lifecycle of
MemorySegments by using specific confinedArenas where possible, and specific index-boundArenas where the lifetime needs to be bound to the lifetime of an index.It addresses all remaining usages of
CuVSResources#arenaafter #1024 and #1045; as such, we can removeCuVSResources#arenacompletely, and force native memory usages to be tracked and dealt with specifically.This would towards the goal of #1037