[GIL] Can image::recreate reuse the memory?

TONGARI

13 Apr 2013 13 Apr '13

4:37 p.m.

Hi, I'd expect image::recreate to reuse the memory much like std::vector::resize, but looked into the code shows that it creates a new one and swaps... So, is there a way to 'resize' the image without reallocating if the internal memory is sufficient? Thanks

Show replies by date

Christian Henning

13 Apr 13 Apr

9:48 p.m.

...

I'd expect image::recreate to reuse the memory much like std::vector::resize, but looked into the code shows that it creates a new one and swaps...

So, is there a way to 'resize' the image without reallocating if the internal memory is sufficient?

I afraid there is not a way right now. Do you have a quick fix or would such behavior be more complicated? Regards, Christian

TONGARI

14 Apr 14 Apr

3:39 a.m.

2013/4/14 Christian Henning

...

...
I'd expect image::recreate to reuse the memory much like std::vector::resize, but looked into the code shows that it creates a new one and swaps...

So, is there a way to 'resize' the image without reallocating if the internal memory is sufficient?

I afraid there is not a way right now. Do you have a quick fix or would such behavior be more complicated?

You can replace the data member '_align_in_bytes' with, say, _capacity. And make those member functions which use _align_in_bytes accept an extra parameter align_in_bytes. When deallocate, you can use _capacity without recomputing the total_allocated_size_in_bytes. As to recreate, you can compute total_allocated_size_in_bytes which gives you the new capacity and compare it with the old _capacity to decide reallocate or not. Just a quick thought, maybe something I missed? Thanks

TONGARI

2:14 p.m.

2013/4/14 TONGARI

...

2013/4/14 Christian Henning

...
...
I'd expect image::recreate to reuse the memory much like std::vector::resize, but looked into the code shows that it creates a

new

...
one and swaps...

So, is there a way to 'resize' the image without reallocating if the internal memory is sufficient?

I afraid there is not a way right now. Do you have a quick fix or would such behavior be more complicated?

You can replace the data member '_align_in_bytes' with, say, _capacity. And make those member functions which use _align_in_bytes accept an extra parameter align_in_bytes. When deallocate, you can use _capacity without recomputing the total_allocated_size_in_bytes.

As to recreate, you can compute total_allocated_size_in_bytes which gives you the new capacity and compare it with the old _capacity to decide reallocate or not.

Just a quick thought, maybe something I missed?

Well, after a deeper look, some ctors need _align_in_bytes from another image... if you don't care an extra data member, it's still doable. Or maybe I should cook me own :(

...

Thanks

Christian Henning

11:15 p.m.

...

Well, after a deeper look, some ctors need _align_in_bytes from another image... if you don't care an extra data member, it's still doable.

Or maybe I should cook me own :(

No need to cook your own. I think this functionality should be supported. For that I have added a resize() in the image, image_view, and some locators classes. I don't have time today to finish the code and commit it but tomorrow after work I should be able to finish the first version. I let you know! Regards, Christian

Christian Henning

11:35 p.m.

On Sun, Apr 14, 2013 at 7:15 PM, Christian Henning wrote:

...

Well, after a deeper look, some ctors need _align_in_bytes from another

...
image... if you don't care an extra data member, it's still doable.

Or maybe I should cook me own :(

No need to cook your own. I think this functionality should be supported. For that I have added a resize() in the image, image_view, and some locators classes. I don't have time today to finish the code and commit it but tomorrow after work I should be able to finish the first version.

I let you know!

Sorry, I don't think I was clear enough. I only added the resize() function so I understand what I need to change. In the final version all the necessary code will be inside recreate(). Christian

Christian Henning

25 Apr 25 Apr

12:13 a.m.

...

On Sun, Apr 14, 2013 at 7:15 PM, Christian Henning wrote:

...
Well, after a deeper look, some ctors need _align_in_bytes from another

...
image... if you don't care an extra data member, it's still doable.

Or maybe I should cook me own :(

I have added a fix to reuse memory when an image is smaller than the older one. Would you mind having a look and give me some feedback? Thanks! Christian

TONGARI

2:44 a.m.

2013/4/25 Christian Henning

...

...
On Sun, Apr 14, 2013 at 7:15 PM, Christian Henning
...
Well, after a deeper look, some ctors need _align_in_bytes from another

...
image... if you don't care an extra data member, it's still doable.

Or maybe I should cook me own :(

I have added a fix to reuse memory when an image is smaller than the older one. Would you mind having a look and give me some feedback?

Unfortunately I think this is buggy for deallocate. Since _view's dimensions are changed after recreated, you can't get the original capacity from the changed dimensions. Capacity must be kept to make deallocate right. BTW, I've made my own image type which accepts align as a template param instead. Thanks

Christian Henning

26 Apr 26 Apr

12:17 a.m.

On Wed, Apr 24, 2013 at 10:44 PM, TONGARI wrote:

...

2013/4/25 Christian Henning

...
...
On Sun, Apr 14, 2013 at 7:15 PM, Christian Henning <

...
wrote:

...
Well, after a deeper look, some ctors need _align_in_bytes from

another

...
...
image... if you don't care an extra data member, it's still doable.

Or maybe I should cook me own :(

I have added a fix to reuse memory when an image is smaller than the

chhenning@gmail.com older one. Would you mind having a look and give me some feedback?

Unfortunately I think this is buggy for deallocate. Since _view's dimensions are changed after recreated, you can't get the original capacity from the changed dimensions.

Capacity must be kept to make deallocate right.

You are absolutely correct! But I think it's enough to keep the original dimensions so the deallocate( ... ) call in the destructors frees the correct amount of memory.

...

BTW, I've made my own image type which accepts align as a template param instead.

I could see this making sense. Is it just exchanging the member with an int template parameter? I'm a bit worries of breaking too much code when introducing into boost. What do you think?

Christian

...

Thanks

_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

Christian Henning

12:52 a.m.

Unfortunately I think this is buggy for deallocate.

...

Since _view's dimensions are changed after recreated, you can't get the

...
original capacity from the changed dimensions.

Capacity must be kept to make deallocate right.

I fixed it. Please have a look when you get a chance. Thanks! Christian

TONGARI

3:30 a.m.

2013/4/26 Christian Henning

...

On Wed, Apr 24, 2013 at 10:44 PM, TONGARI wrote:

...
2013/4/25 Christian Henning

...
...
On Sun, Apr 14, 2013 at 7:15 PM, Christian Henning <

...
wrote:

...
Well, after a deeper look, some ctors need _align_in_bytes from

another

...
...
image... if you don't care an extra data member, it's still doable.

Or maybe I should cook me own :(

I have added a fix to reuse memory when an image is smaller than the

chhenning@gmail.com older one. Would you mind having a look and give me some feedback?

Unfortunately I think this is buggy for deallocate. Since _view's dimensions are changed after recreated, you can't get the original capacity from the changed dimensions.

Capacity must be kept to make deallocate right.

You are absolutely correct! But I think it's enough to keep the original dimensions so the deallocate( ... ) call in the destructors frees the correct amount of memory.

I can't see why you prefer dimensions (2 ints) to capacity (1 size_t), seems to me that using capacity has both space & time efficiency (i.e. no need to recompute total_allocated_size_in_bytes);

...

...
BTW, I've made my own image type which accepts align as a template param instead.

I could see this making sense. Is it just exchanging the member with an int template parameter?

And it stores capacity instead, I also improved the align-function a bit. I'm a bit worries of breaking too much code when

...

introducing into boost. What do you think?

It'd be better not to break the API (not sure if ABI matters) so I didn't propose the change, instead, we could have another one as I did for myself. Regards,

Christian Henning

29 Apr 29 Apr

1:08 a.m.

...

I can't see why you prefer dimensions (2 ints) to capacity (1 size_t), seems to me that using capacity has both space & time efficiency (i.e. no need to recompute total_allocated_size_in_bytes);

For some reasons I was thinking about int overflow but I realized that doesn't make much sense. So, I changed the code to your suggestion.

...

I'm a bit worries of breaking too much code when

...
introducing into boost. What do you think?

It'd be better not to break the API (not sure if ABI matters) so I didn't propose the change, instead, we could have another one as I did for myself.

Yes, ABI changes matter since gil has been part of boost for a couple of years now. I made it note for the next gil version. Regards, Christian

TONGARI

4:23 p.m.

Hi Christian, 2013/4/29 Christian Henning

...

...
I can't see why you prefer dimensions (2 ints) to capacity (1 size_t), seems to me that using capacity has both space & time efficiency (i.e. no need to recompute total_allocated_size_in_bytes);

For some reasons I was thinking about int overflow but I realized that doesn't make much sense. So, I changed the code to your suggestion.

...
I'm a bit worries of breaking too much code when

...
introducing into boost. What do you think?

It'd be better not to break the API (not sure if ABI matters) so I didn't propose the change, instead, we could have another one as I did for

myself.

...
Yes, ABI changes matter since gil has been part of boost for a couple of years now.

I made it note for the next gil version.

I think the code may have align problem in 2 respects: 1) line 336: _memory is not the aligned address which is previously returned by align-function. 2) In 'recreate': You have to update _align_in_bytes before calling create_view BTW, why is the 3rd overload of recreate kept unmodified?

Christian Henning

30 Apr 30 Apr

2:10 p.m.

...

I think the code may have align problem in 2 respects:

1) line 336: _memory is not the aligned address which is previously returned by align-function.

The function create_view is only called when reusing the image's memory. I guess I don't understand what you mean.

...

2) In 'recreate': You have to update _align_in_bytes before calling create_view

When the user recreates an image with a different _align_in_bytes we need to reallocate the memory. I added this condition to the code. If the alignment doesn't change then I don't need to update the member.

...

BTW, why is the 3rd overload of recreate kept unmodified?

Good question! For whatever reason I thought it takes in a memory pointer. But on a closer look it I realize that's nonsense. I have added the new code. Thanks for helping out! Christian

TONGARI

3:26 p.m.

2013/4/30 Christian Henning

...

...
I think the code may have align problem in 2 respects:

1) line 336: _memory is not the aligned address which is previously returned by align-function.

The function create_view is only called when reusing the image's memory. I guess I don't understand what you mean.

For example, look line 313: the memory address 'tmp' is aligned which may differ from '_memory'. So you can't recreate the view with _memory, instead, you should use the aligned address.

...

...
2) In 'recreate': You have to update _align_in_bytes before calling create_view

When the user recreates an image with a different _align_in_bytes we need to reallocate the memory. I added this condition to the code. If the alignment doesn't change then I don't need to update the member.

You don't have to test the equality of _align_in_bytes & _alloc... Even if _align_in_bytes differs, you could still reuse the space. As for _alloc, I'm not sure if you want to guarantee that even if no reallocation is needed, as long as the user provide another allocator, the whole memory should be allocate by it. I'm not sure how a allocator would be implemented, but since it's a default param currently, while the user might just want to use the original one (not the implicitly newly constructed one which may != the orignal), it might surprise the user if he does care about the allocator used, so my suggestion here is to separate it to 2 versions with & without allocator, at least, it won't break the API (I personally would like to drop the Alloc param here...) Last but not least, 'total_allocated_size_in_bytes' also depends on '_align_in_bytes', so nevertheless, you have to update it first. HTH

Christian Henning

1 Jun 1 Jun

3:03 p.m.

Sorry for the very late reply.

...

The function create_view is only called when reusing the image's memory. I

...
guess I don't understand what you mean.

For example, look line 313: the memory address 'tmp' is aligned which may differ from '_memory'.

So you can't recreate the view with _memory, instead, you should use the aligned address.

I understand now. I fixed it.

...

...
...
2) In 'recreate': You have to update _align_in_bytes before calling create_view

When the user recreates an image with a different _align_in_bytes we need to reallocate the memory. I added this condition to the code. If the alignment doesn't change then I don't need to update the member.

You don't have to test the equality of _align_in_bytes & _alloc... Even if _align_in_bytes differs, you could still reuse the space. As for _alloc, I'm not sure if you want to guarantee that even if no reallocation is needed, as long as the user provide another allocator, the whole memory should be allocate by it.

If the new align_in_bytes is larger than the old one it could mean we have to reallocate. Do you agree? I'm not sure how a allocator would be implemented, but since it's a default

...

param currently, while the user might just want to use the original one (not the implicitly newly constructed one which may != the orignal), it might surprise the user if he does care about the allocator used, so my suggestion here is to separate it to 2 versions with & without allocator, at least, it won't break the API (I personally would like to drop the Alloc param here...)

I have implemented two versions now.

...

Last but not least, 'total_allocated_size_in_bytes' also depends on '_align_in_bytes', so nevertheless, you have to update it first.

Done. Regards, Christian

TONGARI

5:57 p.m.

2013/6/1 Christian Henning

...

Sorry for the very late reply.

...
The function create_view is only called when reusing the image's memory. I

...
guess I don't understand what you mean.

For example, look line 313: the memory address 'tmp' is aligned which may differ from '_memory'.

So you can't recreate the view with _memory, instead, you should use the aligned address.

I understand now. I fixed it.

...

...
...
...
2) In 'recreate': You have to update _align_in_bytes before calling create_view

When the user recreates an image with a different _align_in_bytes we need to reallocate the memory. I added this condition to the code. If the alignment doesn't change then I don't need to update the member.

You don't have to test the equality of _align_in_bytes & _alloc... Even if _align_in_bytes differs, you could still reuse the space. As for _alloc, I'm not sure if you want to guarantee that even if no reallocation is needed, as long as the user provide another allocator, the whole memory should be allocate by it.

If the new align_in_bytes is larger than the old one it could mean we have to reallocate. Do you agree?

If total_allocated_size_in_bytes not exceeds, than no need to reallocate. I'm not sure how a allocator would be implemented, but since it's a default

...

...
param currently, while the user might just want to use the original one (not the implicitly newly constructed one which may != the orignal), it might surprise the user if he does care about the allocator used, so my suggestion here is to separate it to 2 versions with & without allocator, at least, it won't break the API (I personally would like to drop the Alloc param here...)

I have implemented two versions now.

Be sure to make a note to the user since the behavior is changed. something like: "if no allocator is provided to 'recreate', it now uses the previous one instead of creating a new one"

...

...
Last but not least, 'total_allocated_size_in_bytes' also depends on '_align_in_bytes', so nevertheless, you have to update it first.

...
Done.

BTW, 'mpl::bool_<IsPlanar>()' may be more brief than 'typename boost::conditional< IsPlanar, mpl::true_, mpl::false_ >::type()' Regards,

Christian Henning

6 Jun 6 Jun

5:57 p.m.

...

Be sure to make a note to the user since the behavior is changed. something like: "if no allocator is provided to 'recreate', it now uses the previous one instead of creating a new one"

I added a comment.

...

...
...
Last but not least, 'total_allocated_size_in_bytes' also depends on '_align_in_bytes', so nevertheless, you have to update it first.

...
Done.

BTW, 'mpl::bool_<IsPlanar>()' may be more brief than 'typename boost::conditional< IsPlanar, mpl::true_, mpl::false_ >::type()'

Done. Thanks! Christian

4191

Age (days ago)

4245

Last active (days ago)

List overview

Download

17 comments

2 participants

participants (2)

Christian Henning
TONGARI