Re: [PR] [SYSTEMDS-3867] Safe Fast Frame Copy [systemds]

2026-02-02 Thread via GitHub 


janniklinde commented on PR #2399:
URL: https://github.com/apache/systemds/pull/2399#issuecomment-3833811942

   Thank you for your analysis and contribution @yj-40-56. It's good to know 
that you identified `OptionalArray`, `BitSetArray`, `RaggedArray`, and 
`CompressedArray` as array types that may require locking. I think it would be 
safer to allow a whitelist of array types to bypass locking (also for future 
proofing). I'll leave the PR open to fix some formatting issues and revisit 
some aspects that could be done more efficiently. Good luck in your exam!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] [SYSTEMDS-3867] Safe Fast Frame Copy [systemds]

2026-01-28 Thread via GitHub 


yj-40-56 commented on PR #2399:
URL: https://github.com/apache/systemds/pull/2399#issuecomment-3814017427

   Replaced Locking on ValueType.BOOLEAN with locking based on ArrayType:
   
   - BitSetArray requires locking due to underlying storage in longs. Small 
Array are safe if they are shorter than 64 rows (using underlying 
Byte[]) a. Once above that threshold, the array is turned into a BitSetArray
   
   - OptionalArray has an underlying null-mask, indicating the null entries. 
That null-mask is based on a BitSetArray and is not thread-safe.
   
   Additional ArrayTypes that are locked:
   - RaggedArray: internal resizing _should_ cause race conditions. The 
set()/append() methods all contained a reset() into set() segment which has no 
guarantee of being atomic.
   - DDCArray/ACompressedArray: Similar to RaggedArrays, modifications are not 
atomic.
   
   
   I have verified the fix for BitSetSArray and OptionalArray.
   
   I did not include tests for RaggedArray and ACompressedArray. After looking 
at the implementations, I am  convinced they are not thread-safe, but I was not 
able to create unit tests to show that specific behaviour. 
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]