Skip to content

remove one Vec from Tensor - #31

Open
Human9000-bit wants to merge 1 commit into
enthropy7:mainfrom
Human9000-bit:tensor-inline-strides
Open

remove one Vec from Tensor#31
Human9000-bit wants to merge 1 commit into
enthropy7:mainfrom
Human9000-bit:tensor-inline-strides

Conversation

@Human9000-bit

Copy link
Copy Markdown
Collaborator

I really disliked Vecs out there in tensor, so I inlined strides.

Benches: tensor ops -8% avg, onnx bench -3% avg

compute_strides() returned Option<Vec<usize>>, so every Tensor
constructor -- from_vec, from_aligned, from_f16, from_bf16, filled,
zeros, reshape, into_reshape -- heap-allocated a three-to-five element
Vec, had DimsVec::from memcpy it into the inline buffer, and freed it
again. DimsVec exists precisely to keep shape and strides off the heap
for rank <= 6, so the stride path was paying one malloc/free per tensor
to populate a structure designed to avoid exactly that.

compute_strides now fills a [usize; INLINE_CAP] and returns the DimsVec
directly, keeping the old vec! path only for rank > 6, where DimsVec
spills to Heap anyway. The eight call sites move the value into the
struct instead of converting it. DimsVec itself is untouched: rank > 6
still works and size_of::<Tensor>() is still 144 bytes.

A first attempt that wrote the strides through a DerefMut on DimsVec was
slower than the Vec it replaced (37.8 ns/op on the reshape bench) because
every index write re-matched the enum discriminant. The win needs the
plain array, so keep it that way.

Validation: cargo test -p yscv-tensor, 228 passed; scripts/check-ci-local.sh
passed. Microbench on a 12th Gen Core i5-12500H, pinned with taskset -c 2,
performance governor, min of five interleaved runs against the unpatched
binary: reshape of a 4-D tensor 33.6 -> 30.9 ns/op (-8 %), from_vec of a
rank-3 tensor 51.0 -> 48.0 ns/op (-6 %), clone unchanged at 13.2 ns/op.
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.

1 participant