Add shrink_to_fit to Array

[bokutotu]

#427

• add shrink_to_fit to Array<A, D>
• add to_default_strid to Array<A, D>
• add offset_from_data to Array<A, D>
• add shrink_to_fit to OwnedRepr

[bluss]

to default stride is an expensive operation - so we can probably avoid it on an array that is already perfectly fit.

We do prefer to not lean too strongly on a default memory order if possible. Any contiguous array should not have its memory order thrown around. I would even prefer if an array keeps its memory order the same, if it can.

[bluss]

I'm not sure about all the details of this method, but I think it's better to take a step back and see if there isn't already some functionality that can do this - maybe .to_shape()?

[bluss]

shrink_to_fit is a good idea - should it modify array memory layout? When should it modify? Some more description could help us get a good design.

[bokutotu]

Thanks for your the code review 😀

shrink_to_fit is a good idea - should it modify array memory layout? When should it modify? Some more description could help us get a good design.

For example, I think that changing the memory order is necessary in the following cases.

let a = array![[[0, 1, 2], [3, 4, 5], [6, 7, 8]], [[9, 10, 11], [12, 13, 14], [15, 16, 17]], [[18, 19, 20], [21, 22, 23], [24, 25, 26]]];
let a = a.slice_move(s![.., 1.., ..]);
// a -> 
//   [[[3, 4, 5],
//   [6, 7, 8]],
// 
// [[12, 13, 14],
//  [15, 16, 17]],
//
// [[21, 22, 23],
//  [24, 25, 26]]], shape=[3, 2, 3], strides=[9, 3, 1], layout=c (0x4), const ndim=3

I think the memory order would be as follows if we apply shrink_to_fit to this array.

before shrink_to_fit

0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26

after shrink_to_fit

3 4 5 6 7 8 12 13 14 15 16 17 21 22 23 24 25 26

On the other hand, in the following case, it looks like you can do shrink_to_fit without changing the memory order, and just release a part of the heap.

let a = array![[[0, 1, 2], [3, 4, 5], [6, 7, 8]], [[9, 10, 11], [12, 13, 14], [15, 16, 17]], [[18, 19, 20], [21, 22, 23], [24, 25, 26]]];
let a = a.slice_move(s![..2, .., ..]);
// a -> 
//   [[[0, 1, 2],
//  [3, 4, 5],
//  [6, 7, 8]],
//
// [[9, 10, 11],
//  [12, 13, 14],
//  [15, 16, 17]]], shape=[2, 3, 3], strides=[9, 3, 1], layout=Cc (0x5), const ndim=3

I think the only time you can apply shrink_to_fit without changing the memory order is when the dimension with the largest stride is changed, and there is no offset from self.data.ptr to self.ptr. So I think shrink_to_fit without changing the memory order can only be used for quite limited cases.

[bluss]

I'm back next week

[bluss]

Apparently not back so fast, I apologize.

[bokutotu]

Don't worry about it. We all have busy times 😀

[bokutotu]

What is the current status of this PR?

[bluss]

Well, I want to come back to this and it would be the first thing I'd work on in ndarray

[adamreichold]

@bokutotu If I you are still interested, I would like to try to pick this up again.

What would be really helpful would be to remove the formatting-related part of the diff, just so that is easier to review.

Other than that, I suspect that all of the feedback provided so far has already been addressed.

If you do reboot this, please try to ignore all the Clippy and deprecation issues for now. We will have to get the master branch into shape elsewhere.

[bokutotu]

@adamreichold Thanks for your reply. I would like to make a commit to delete the diffs once they are out in cargo fmt. Please wait a bit as work is taking up a lot of my time right now.