fix(lang): truthful ownership labels in TensorStorageFactory (#927) - #935
Merged
Merged
Conversation
borrowFloatArray promised '(zero-copy)' and returned a Borrowed handle, but a FloatArray has no byte-view in common Kotlin — the implementation re-encoded every float into a private ByteArray and labeled the copy Borrowed. That misrepresents ownership to every consumer of TensorStorage.ownership and to MemoryTracker reports. It is now deprecated (ReplaceWith fromFloatArray) and honestly returns Owned. fromTensorData's KDoc claimed 'the underlying data is borrowed (not copied)' while the dense float/int branches copy via BufferHandleFactory.owned. The contract is now documented per branch: packed Q4_K/Q8_0 borrow genuinely zero-copy (shared packedData), dense arrays convert to owned bytes, everything else materializes. Tests: the acceptance test that enshrined the false promise (ac3_borrowedConstructorDoesNotCopy) now asserts the honest OWNED label; a new ac3 test shows where real borrowing lives (fromRawBytes, with mutation visibility through the shared array); the fromTensorData bridge tests gain ownership assertions plus a packed zero-copy mutation-visibility test. Closes #927
|
📖 Documentation Preview The documentation has been built successfully for this PR. Generated Files:
Artifacts:
This comment will be updated automatically when the PR is updated. |
|
📖 Documentation Preview The documentation has been built successfully for this PR. Generated Files:
Artifacts:
This comment will be updated automatically when the PR is updated. |
|
📖 Documentation Preview The documentation has been built successfully for this PR. Generated Files:
Artifacts:
This comment will be updated automatically when the PR is updated. |
aharakal
previously approved these changes
Aug 10, 2026
|
📖 Documentation Preview The documentation has been built successfully for this PR. Generated Files:
Artifacts:
This comment will be updated automatically when the PR is updated. |
aharakal
approved these changes
Aug 10, 2026
|
📖 Documentation Preview The documentation has been built successfully for this PR. Generated Files:
Artifacts:
This comment will be updated automatically when the PR is updated. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Full analysis in #927. Two contract violations, one fix strategy: make the labels tell the truth, because
BufferHandle.Ownershipis the storage layer's authority andMemoryTrackerreports key off it.borrowFloatArray— doc said "(zero-copy)", implementation re-encoded floats into a privateByteArrayand labeled the copyBorrowed. AFloatArrayhas no byte-view in common Kotlin, so genuine borrowing is impossible here by construction. Now deprecated withReplaceWith(fromFloatArray)and honestly returnsOwned. Real zero-copy borrowing lives infromRawBytes(byte input), which the docs now point to.fromTensorData— doc said "borrowed (not copied)" while the dense branches copy. The KDoc now states the per-branch contract: packed Q4_K/Q8_0 borrow genuinely zero-copy (sharedpackedData), dense arrays convert to owned bytes, everything else materializes.Tests: the acceptance test that enshrined the false promise (
ac3_borrowedConstructorDoesNotCopy) is replaced by the honest assertion; new mutation-visibility tests pin both the genuine-borrow path (fromRawBytes) and the packed zero-copy bridge.Verified:
:skainet-lang:skainet-lang-core:jvmTestgreen (full module).Note: CHANGELOG entry included — expect trivial
[Unreleased]conflicts with sibling PRs in the #927–#931 series.Closes #927