xmlvm-users

[xmlvm-users] problem with arrays

[Panayotis K. <pan...@pa...>]

Hello!

I think I found a problem with the current implementation of XMLVM and java arrays.
I am talking about this:

+ (XMLVMArray*) createSingleDimensionWithType:(int) type size:(int) size andData:(void*) data;

In the currently implementation, data is not retained or copied anywhere. Thus, if data was automatically released at some time, the array has serious problems (and it took me a whole day to find it). 
The problem appears for example when NSData.bytes(); is used, where data is released back to the system.

I can think of two possible solutions to this problem:
1) create a memory copy of the original data (i.e. do something similar to what clone__ does)
2) add a new member variable to XMLVMArray, which will keep hold of the creator object, and retain it (and release it when dealloc of array is found)

Since this method is only used with NSData and AFAICS with initializing of multi-dimension arrays with "new" (only for their first dimension), and since NSData clearly states that this is a const structure, while with Java you can more or less do whatever you want with it, I'd suggest to fix this issue with the first solution.
When we agree, I could submit a patch.

What do you think?

Re: [xmlvm-users] problem with arrays

[Arno P. <ar...@pu...>]

I agree in principle with your proposed first solution. However, there 
are also instances where it is not necessary to copy the array (e.g., 
the dex:fill-array instruction). I would suggest to add another boolean 
parameter to that method that states if 'data' should be copied or not 
and then go through the code and decide individually where the data 
needs to be copied or not. If we can avoid unnecessary copying, we should.

Arno


On 12/16/10 10:37 AM, Panayotis Katsaloulis wrote:
> Hello!
>
> I think I found a problem with the current implementation of XMLVM and java arrays.
> I am talking about this:
>
> + (XMLVMArray*) createSingleDimensionWithType:(int) type size:(int) size andData:(void*) data;
>
> In the currently implementation, data is not retained or copied anywhere. Thus, if data was automatically released at some time, the array has serious problems (and it took me a whole day to find it).
> The problem appears for example when NSData.bytes(); is used, where data is released back to the system.
>
> I can think of two possible solutions to this problem:
> 1) create a memory copy of the original data (i.e. do something similar to what clone__ does)
> 2) add a new member variable to XMLVMArray, which will keep hold of the creator object, and retain it (and release it when dealloc of array is found)
>
> Since this method is only used with NSData and AFAICS with initializing of multi-dimension arrays with "new" (only for their first dimension), and since NSData clearly states that this is a const structure, while with Java you can more or less do whatever you want with it, I'd suggest to fix this issue with the first solution.
> When we agree, I could submit a patch.
>
> What do you think?
>
>
> ------------------------------------------------------------------------------
> Lotusphere 2011
> Register now for Lotusphere 2011 and learn how
> to connect the dots, take your collaborative environment
> to the next level, and enter the era of Social Business.
> http://p.sf.net/sfu/lotusphere-d2d
> _______________________________________________
> xmlvm-users mailing list
> xml...@li...
> https://lists.sourceforge.net/lists/listinfo/xmlvm-users

Re: [xmlvm-users] problem with arrays

[Panayotis K. <pan...@pa...>]

On Dec 17, 2010, at 2:19 AM, Arno Puder wrote:

> 
> I agree in principle with your proposed first solution. However, there 
> are also instances where it is not necessary to copy the array (e.g., 
> the dex:fill-array instruction). I would suggest to add another boolean 
> parameter to that method that states if 'data' should be copied or not 
> and then go through the code and decide individually where the data 
> needs to be copied or not. If we can avoid unnecessary copying, we should.
> 
> Arno

You mean this one?
dex:filled-new-array|dex:filled-new-array-range
in line 3500 of xmlvm2objc.xsl
 
I am not at all experienced with dex, and I have a question!
From my understanding this is the only location where createSingleDimensionWithType...andData is called from the ObjC backend.
And there is only one more, called by NSData.
So, should I replace this generic call of the function with a new one, passing "copyData:NO" for the dex instruction and "copyData:Yes" for NSData ?

I am trying to understand the memory management of the produced dex instruction. Why is no copying of data required at that point?

Re: [xmlvm-users] problem with arrays

[Panayotis K. <pan...@pa...>]

I have created a new patch about it, please have a look here:

http://xmlvm-reviews.appspot.com/102001

Re: [xmlvm-users] problem with arrays

[Arno P. <ar...@pu...>]

looks good, however, you made an interesting point by saying that there 
are no const arrays in Java. I think the current implementation of 
fill-array is not correct: the data needs to be copied there as well in 
case the application modifies the initial values.

I think the data needs to be copied in any case and the flag ownsData 
should always be TRUE (meaning, it can be removed).

Arno


On 12/17/10 2:10 AM, Panayotis Katsaloulis wrote:
> I have created a new patch about it, please have a look here:
>
> http://xmlvm-reviews.appspot.com/102001
>
> ------------------------------------------------------------------------------
> Lotusphere 2011
> Register now for Lotusphere 2011 and learn how
> to connect the dots, take your collaborative environment
> to the next level, and enter the era of Social Business.
> http://p.sf.net/sfu/lotusphere-d2d
> _______________________________________________
> xmlvm-users mailing list
> xml...@li...
> https://lists.sourceforge.net/lists/listinfo/xmlvm-users

Re: [xmlvm-users] problem with arrays

[Panayotis K. <pan...@pa...>]

On Dec 18, 2010, at 6:34 AM, Arno Puder wrote:

> 
> looks good, however, you made an interesting point by saying that there 
> are no const arrays in Java. I think the current implementation of 
> fill-array is not correct: the data needs to be copied there as well in 
> case the application modifies the initial values.
> 
> I think the data needs to be copied in any case and the flag ownsData 
> should always be TRUE (meaning, it can be removed).
> 
> Arno

... which also means, that XMLVMArray->ownsData should be removed, since there is no method to set it to NO ? 

Re: [xmlvm-users] problem with arrays

[Arno P. <ar...@pu...>]

yep

On 12/17/10 11:29 PM, Panayotis Katsaloulis wrote:
>
> On Dec 18, 2010, at 6:34 AM, Arno Puder wrote:
>
>>
>> looks good, however, you made an interesting point by saying that there
>> are no const arrays in Java. I think the current implementation of
>> fill-array is not correct: the data needs to be copied there as well in
>> case the application modifies the initial values.
>>
>> I think the data needs to be copied in any case and the flag ownsData
>> should always be TRUE (meaning, it can be removed).
>>
>> Arno
>
> ... which also means, that XMLVMArray->ownsData should be removed, since there is no method to set it to NO ?
> ------------------------------------------------------------------------------
> Lotusphere 2011
> Register now for Lotusphere 2011 and learn how
> to connect the dots, take your collaborative environment
> to the next level, and enter the era of Social Business.
> http://p.sf.net/sfu/lotusphere-d2d
> _______________________________________________
> xmlvm-users mailing list
> xml...@li...
> https://lists.sourceforge.net/lists/listinfo/xmlvm-users

Re: [xmlvm-users] problem with arrays

[Panayotis K. <pan...@pa...>]

I think I found another problem with arrays.

When using 
XMLVMArray createSingleDimensionWithType:size:andData
in a multi-dimension array, it clearly states that each entry of the array is of type XMLVMElem, (which is of size 8), while the type (and thus the type size) could be smaller.
I think the implementation of this method should change, so that it could "trim" the data when this method is called automatically and to convert it to the correct size for each element.

I have a question here: is this method automatically inserted in the source code *only* for the multi-dimensional arrays, in order to define the higher orders of array length? 

Re: [xmlvm-users] problem with arrays

[Panayotis K. <pan...@pa...>]

On Dec 19, 2010, at 3:06 PM, Panayotis Katsaloulis wrote:

> I think I found another problem with arrays.
> 
> When using 
> XMLVMArray createSingleDimensionWithType:size:andData
...

I think I found a solution: this method is only called from 
<xsl:template match="dex:filled-new-array|dex:filled-new-array-range">
which always passes data as XMLVMElem[]

I believe there should be a special selector just for this case, something like
XMLVMArray createSingleDimensionWithType:size:andXMLVMElem
which will hint the runtime that this is not raw data of the given type,  but of XMLVMElem.
So it will ignore the size of the given type and use size of XMLVMElem instead. 

Re: [xmlvm-users] problem with arrays

[Panayotis K. <pan...@pa...>]

On Dec 19, 2010, at 5:12 PM, Panayotis Katsaloulis wrote:

> 
> On Dec 19, 2010, at 3:06 PM, Panayotis Katsaloulis wrote:
> 
>> I think I found another problem with arrays.
>> 
>> When using 
>> XMLVMArray createSingleDimensionWithType:size:andData
> ...
> 
> I think I found a solution: this method is only called from 
> <xsl:template match="dex:filled-new-array|dex:filled-new-array-range">
> which always passes data as XMLVMElem[]
> 
> I believe there should be a special selector just for this case, something like
> XMLVMArray createSingleDimensionWithType:size:andXMLVMElem
> which will hint the runtime that this is not raw data of the given type,  but of XMLVMElem.
> So it will ignore the size of the given type and use size of XMLVMElem instead. 
> 


I am hoping I am not bombing this list with this issue :)

After some thoughts and tests, I think I found the optimum solution:
since this is a special case of table creation, which contains items of different data than usual, I believe the most elegant solution is to create a new "type" of XMLVMArray which holds XMLVMElem items, so that we can be safe & fast at the same time.

The patch is  (among with a couple of other smaller fixes) here:
http://xmlvm-reviews.appspot.com/103001

Please have a look!