sahib opened a new pull request, #326:
URL: https://github.com/apache/arrow-go/pull/326

   ### Rationale for this change
   
   This is a follow-up to https://github.com/apache/arrow-go/pull/323.
   
   ARM 32-bit requires atomic operations to be 32-bit aligned. This has not 
been the case in a rather large amount of cases in the code base. Failing to 
align causes crashes during runtime.
   
   ### What changes are included in this PR?
   
   I replaced all uses of `atomic.LoadInt64`, `atomic.StoreInt64` and 
`atomic.AddInt64` with `atomic.Int64{}` and its methods. This type has built-in 
alignment, so it does not matter in which order struct fields appear, which 
makes it generally harder to screw up alignment during changes.
   
   ### Are these changes tested?
   
   Briefly on the 32-bit architecture with the same program mentioned in #323. 
The best way to move forward here would be to also add a 32bit CI environment 
to make sure `arrow` builds and tests there. But I guess this is out of scope 
for this PR.
   
   I've noticed that `go vet ./...` produces many warnings over the project, 
most of them already there before this PR. The new ones are all of the kind:
   
   ```
   arrow/array/dictionary.go:614:42: literal copies lock value from bldr: 
github.com/apache/arrow-go/v18/arrow/array.dictionaryBuilder contains 
github.com/apache/arrow-go/v18/arrow/array.builder contains sync/atomic.Int64 
contains sync/atomic.noCopy
   ```
   
   I.e. false positives due to initialization in another struct and then 
copying it in a constructor function.
   
   ### Are there any user-facing changes?
   
   In the hope that everything was done right: no.


-- 
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]

Reply via email to