Skip to content
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

1.0 #138

Merged
merged 75 commits into from
Sep 13, 2023
Merged

1.0 #138

merged 75 commits into from
Sep 13, 2023

Conversation

novacrazy
Copy link
Collaborator

@novacrazy novacrazy commented Mar 26, 2023

With #137, this opens up the opportunity for a full 1.0 release with much-improved ergonomics.

CHANGELOG.md

What other changes should we make while we can?

Closes #145
Closes #140
Closes #137
Closes #135
Closes #132
Closes #119
Closes #117
Closes #109
Closes #107
Closes #101

Additional tweaks:

  • Hex encoding uses a single buffer for N <= 1024 rather than N < 1024
  • Hex encoding optimizations to avoid bounds checks and remove code duplication
  • Tweaked where actual as_slice/deref/as_ref casts occur, with more inlining
  • Simplified doc comments (removed fn main(), etc.)
  • Adjusts readme to reflect modern Rust with min_const_generics
  • Added an additional check inside the private const-transmute function to verify type sizes.
  • Made the ArrayType implementations Sealed + unreachable Clone methods
  • Added pre-iteration checks for FromIterator and deserialization routines
  • Re-Optimized specialized zip routines for non-Drop items
  • Added const variants of fallible APIs (try_from_slice)
  • Added GenericArray::len() associated method without &self

Remaining todos:

@novacrazy novacrazy mentioned this pull request Mar 26, 2023
@Qqwy
Copy link

Qqwy commented Mar 27, 2023

These changes are great!

I have pointed my local project to the 1.0 branch and many trait implementations now become a lot simpler.

What other changes should we make while we can?

Would it be possible to extend the arr! macro to also except a form without a type at the start?

Currently it supports only let foo = arr![usize; 1, 2, 3] and errors on e.g. let foo = arr![1, 2, 3].
I think that in many situations, the explicit type is not neccesary because it might be inferred from the surroundings regardless.
Furthermore, it seems that currently as is used on elements not already matching the type rather than Into::into(), which breaks for any non-primitive types.

Let me know if this is the right place for such a request, or if you want me to create a separate issue for this.

@tarcieri
Copy link

#133 would be very helpful for several of us

@novacrazy
Copy link
Collaborator Author

Would it be possible to extend the arr! macro to also accept a form without a type at the start?

Yes, quite easily. I'm considering making that the default, as arr![] is a tad annoying with the type and using value as T internally. Would probably be better to mirror vec![] in that regard, and simply force users to specify the type when assigning to a variable, so:

let value = arr![1u8, 2, 3, 4]; // GenericArray<u8, U4>
let value = arr![1, 2, 3, 4]; // GenericArray<i32, U4>, because i32 is the default integer type
let value = arr![1f32; U16]; // GenericArray<f32, U16>
let value: GenericArray<usize, _> = arr![4, 5, 6, 7]; // GenericArray<usize, U4>

@novacrazy
Copy link
Collaborator Author

I have changed the arr![] syntax. It is now more simplified and predictable. Changing 98% of the examples and tests required nothing more than removing the previous explicit type syntax or giving it the explicit type with the values themselves (e.g. 1u8), similarly to how vec![] would work. No more internal as casts. Inference just works.

If there is opposition to this change, I can always revert it, but I feel like it's an improvement.

@novacrazy
Copy link
Collaborator Author

novacrazy commented Mar 28, 2023

I've been experimenting with implementing GenericSequence and FunctionalSequence for Box<GenericArray<T, N>> as a way to manipulate/generate heap-allocated generic-arrays. It kind of works, but ends up incompatible with stack-allocated arrays, so heap_array.zip(stack_array, ...) doesn't work. That may be acceptable, though.

It does turn out I can cheat and use a Vec<T> (with fixed capacity) to build up a Box<GenericArray<T, N>> on the heap, then do some pointer casting. No need for a heap-specific ArrayBuilder.

@novacrazy
Copy link
Collaborator Author

After some testing, it seems safe enough to expose GenericSequence and FunctionalSequence for Box<GenericArray<T, N>>.

Could possibly provide a solution to #108 by avoiding the stack entirely for very large (but fixed-size) arrays. Diminishing returns in usefulness of generic-array at that scale.

@novacrazy
Copy link
Collaborator Author

After idly looking through the docs, I've decided to remove TryFromIterError, opting instead to use the single LengthError.

@novacrazy
Copy link
Collaborator Author

Would anyone else subscribed to this PR care to look over the code? More eyes the better.

@novacrazy
Copy link
Collaborator Author

novacrazy commented Sep 3, 2023

I thought I'd fixed this years ago, but as it was there were some obvious autovectorization misses with .zip() especially.

For example, before my latest commits, this code:

pub fn add64(x: GenericArray<f32, U64>, y: GenericArray<f32, U64>) -> GenericArray<f32, U64> {
    x.zip(y, |a, b| a + b)
}

would result in 64 serial vaddss instructions, with 2 moves for each, which was absurd.

Now it correctly autovectorizes to 8 vaddps AVX instructions, even without codegen-units = 1 in my tests.

EDIT: This may have been a regression in Rust/LLVM itself, as Rust 1.65.0 generates basically the same assembly for both inverted_zip branches, and it's only slightly less optimized than current Rust/LLVM with the new fixed code. Not great, not terrible.

Copy link
Owner

@fizyk20 fizyk20 left a comment

Choose a reason for hiding this comment

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

Looks good to me in general, just a few small comments. Thanks again for all your hard work on this crate!

src/arr.rs Outdated Show resolved Hide resolved
src/iter.rs Outdated Show resolved Hide resolved
src/iter.rs Outdated Show resolved Hide resolved
@novacrazy novacrazy merged commit 8ca9559 into master Sep 13, 2023
3 checks passed
@novacrazy
Copy link
Collaborator Author

generic-array 1.0.0 has been published!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment