From 646a85e6ffdd3dfc47b9df5086f2ba6bb862b301 Mon Sep 17 00:00:00 2001 From: Matthew Topol Date: Sun, 3 Oct 2021 18:28:03 -0400 Subject: [PATCH] ARROW-14206: [Go][CI] Fix build on s390x and ARM Closes #11299 from zeroshade/arrow-14206-go-fix-builds Authored-by: Matthew Topol Signed-off-by: Matthew Topol --- ci/scripts/go_test.sh | 4 ++++ go/arrow/array/concat_test.go | 2 +- go/arrow/bitutil/bitmaps.go | 16 ++++++++----- go/arrow/cdata/cdata.go | 27 +++++++++++++--------- go/arrow/cdata/cdata_test.go | 6 ++--- go/arrow/cdata/cdata_test_framework.go | 6 ++--- go/parquet/internal/utils/min_max_arm64.go | 26 +++++++++++++++++++++ go/parquet/internal/utils/min_max_s390x.go | 26 +++++++++++++++++++++ 8 files changed, 89 insertions(+), 24 deletions(-) create mode 100644 go/parquet/internal/utils/min_max_arm64.go create mode 100644 go/parquet/internal/utils/min_max_s390x.go diff --git a/ci/scripts/go_test.sh b/ci/scripts/go_test.sh index 568a572752c9e..f7b2cd963fc4d 100755 --- a/ci/scripts/go_test.sh +++ b/ci/scripts/go_test.sh @@ -29,6 +29,10 @@ case "$(uname)" in ;; esac +if [[ "$(go env GOHOSTARCH)" = "s390x" ]]; then + testargs="" # -race not supported on s390x +fi + pushd ${source_dir}/arrow TAGS="assert,test" diff --git a/go/arrow/array/concat_test.go b/go/arrow/array/concat_test.go index 9e6ab4a138a90..6cc27215a0be6 100644 --- a/go/arrow/array/concat_test.go +++ b/go/arrow/array/concat_test.go @@ -275,7 +275,7 @@ func (cts *ConcatTestSuite) TestCheckConcat() { cts.NoError(err) defer actual.Release() - cts.True(array.ArrayEqual(expected, actual)) + cts.Truef(array.ArrayEqual(expected, actual), "expected: %s\ngot: %s\n", expected, actual) if len(actual.Data().Buffers()) > 0 { if actual.Data().Buffers()[0] != nil { cts.checkTrailingBitsZeroed(actual.Data().Buffers()[0], int64(actual.Len())) diff --git a/go/arrow/bitutil/bitmaps.go b/go/arrow/bitutil/bitmaps.go index 5ebe1aec8649e..4f72ae6bda979 100644 --- a/go/arrow/bitutil/bitmaps.go +++ b/go/arrow/bitutil/bitmaps.go @@ -26,15 +26,19 @@ import ( // helper function to handle big-endian architectures properly var toFromLEFunc func(uint64) uint64 +var byteZero int func init() { if endian.IsBigEndian { // if we're on a big endian architecture, then use the reverse bytes // function so we can perform byte-swaps when necessary toFromLEFunc = bits.ReverseBytes64 + + byteZero = 7 } else { // identity function if we're on a little endian architecture toFromLEFunc = func(in uint64) uint64 { return in } + byteZero = 0 } } @@ -217,7 +221,7 @@ func NewBitmapWordReader(bitmap []byte, offset, length int) *BitmapWordReader { if bm.nwords > 0 { bm.curword = toFromLEFunc(endian.Native.Uint64(bm.bitmap)) } else { - bm.curword = toFromLEFunc(uint64(bm.bitmap[0])) + (*[8]byte)(unsafe.Pointer(&bm.curword))[byteZero] = bm.bitmap[0] } return bm } @@ -276,12 +280,12 @@ func (bm *BitmapWordReader) NextTrailingByte() (val byte, validBits int) { bm.bitmap = bm.bitmap[1:] nextByte := bm.bitmap[0] - val = (*[8]byte)(unsafe.Pointer(&bm.curword))[0] + val = (*[8]byte)(unsafe.Pointer(&bm.curword))[byteZero] if bm.offset != 0 { val >>= byte(bm.offset) val |= nextByte << (8 - bm.offset) } - (*[8]byte)(unsafe.Pointer(&bm.curword))[0] = nextByte + (*[8]byte)(unsafe.Pointer(&bm.curword))[byteZero] = nextByte bm.trailingBits -= 8 bm.trailingBytes-- validBits = 8 @@ -317,7 +321,7 @@ func NewBitmapWordWriter(bitmap []byte, start, len int) *BitmapWordWriter { if ret.len >= int(unsafe.Sizeof(uint64(0))*8) { ret.currentWord = toFromLEFunc(endian.Native.Uint64(ret.bitmap)) } else if ret.len > 0 { - ret.currentWord = toFromLEFunc(uint64(ret.bitmap[0])) + (*[8]byte)(unsafe.Pointer(&ret.currentWord))[byteZero] = ret.bitmap[0] } } return ret @@ -340,7 +344,7 @@ func (bm *BitmapWordWriter) PutNextWord(word uint64) { // +-------------+-----+-------------+-----+ // |<------ next ----->|<---- current ---->| word = (word << uint64(bm.offset)) | (word >> (int64(sz*8) - int64(bm.offset))) - next := endian.Native.Uint64(bm.bitmap[sz:]) + next := toFromLEFunc(endian.Native.Uint64(bm.bitmap[sz:])) bm.currentWord = (bm.currentWord & bm.bitMask) | (word &^ bm.bitMask) next = (next &^ bm.bitMask) | (word & bm.bitMask) endian.Native.PutUint64(bm.bitmap, toFromLEFunc(bm.currentWord)) @@ -355,7 +359,7 @@ func (bm *BitmapWordWriter) PutNextWord(word uint64) { // PutNextTrailingByte writes the number of bits indicated by validBits from b to // the bitmap. func (bm *BitmapWordWriter) PutNextTrailingByte(b byte, validBits int) { - curbyte := (*[8]byte)(unsafe.Pointer(&bm.currentWord))[0] + curbyte := (*[8]byte)(unsafe.Pointer(&bm.currentWord))[byteZero] if validBits == 8 { if bm.offset != 0 { b = (b << bm.offset) | (b >> (8 - bm.offset)) diff --git a/go/arrow/cdata/cdata.go b/go/arrow/cdata/cdata.go index 1d4b5a48333d6..75eb016dc6f02 100644 --- a/go/arrow/cdata/cdata.go +++ b/go/arrow/cdata/cdata.go @@ -22,12 +22,12 @@ package cdata // #include "arrow/c/abi.h" // #include "arrow/c/helpers.h" -// +// #include // int stream_get_schema(struct ArrowArrayStream* st, struct ArrowSchema* out) { return st->get_schema(st, out); } // int stream_get_next(struct ArrowArrayStream* st, struct ArrowArray* out) { return st->get_next(st, out); } // const char* stream_get_last_error(struct ArrowArrayStream* st) { return st->get_last_error(st); } -// struct ArrowArray get_arr() { struct ArrowArray arr; return arr; } -// struct ArrowArrayStream get_stream() { struct ArrowArrayStream stream; return stream; } +// struct ArrowArray* get_arr() { return (struct ArrowArray*)(malloc(sizeof(struct ArrowArray))); } +// struct ArrowArrayStream* get_stream() { return (struct ArrowArrayStream*)malloc(sizeof(struct ArrowArrayStream)); } // import "C" @@ -306,8 +306,7 @@ func (imp *cimporter) doImportChildren() error { } func (imp *cimporter) initarr() { - arr := C.get_arr() - imp.arr = &arr + imp.arr = C.get_arr() } // import is called recursively as needed for importing an array and its children @@ -321,11 +320,14 @@ func (imp *cimporter) doImport(src *CArrowArray) error { defer func(arr *CArrowArray) { if imp.data != nil { runtime.SetFinalizer(imp.data, func(*array.Data) { + defer C.free(unsafe.Pointer(arr)) C.ArrowArrayRelease(arr) if C.ArrowArrayIsReleased(arr) != 1 { panic("did not release C mem") } }) + } else { + C.free(unsafe.Pointer(arr)) } }(imp.arr) @@ -527,10 +529,12 @@ func importCArrayAsType(arr *CArrowArray, dt arrow.DataType) (imp *cimporter, er } func initReader(rdr *nativeCRecordBatchReader, stream *CArrowArrayStream) { - st := C.get_stream() - rdr.stream = &st + rdr.stream = C.get_stream() C.ArrowArrayStreamMove(stream, rdr.stream) - runtime.SetFinalizer(rdr, func(r *nativeCRecordBatchReader) { C.ArrowArrayStreamRelease(r.stream) }) + runtime.SetFinalizer(rdr, func(r *nativeCRecordBatchReader) { + C.ArrowArrayStreamRelease(r.stream) + C.free(unsafe.Pointer(r.stream)) + }) } // Record Batch reader that conforms to arrio.Reader for the ArrowArrayStream interface @@ -560,14 +564,15 @@ func (n *nativeCRecordBatchReader) Read() (array.Record, error) { } arr := C.get_arr() - errno := C.stream_get_next(n.stream, &arr) + defer C.free(unsafe.Pointer(arr)) + errno := C.stream_get_next(n.stream, arr) if errno != 0 { return nil, n.getError(int(errno)) } - if C.ArrowArrayIsReleased(&arr) == 1 { + if C.ArrowArrayIsReleased(arr) == 1 { return nil, io.EOF } - return ImportCRecordBatchWithSchema(&arr, n.schema) + return ImportCRecordBatchWithSchema(arr, n.schema) } diff --git a/go/arrow/cdata/cdata_test.go b/go/arrow/cdata/cdata_test.go index e5e43deddb732..d29bf043636da 100644 --- a/go/arrow/cdata/cdata_test.go +++ b/go/arrow/cdata/cdata_test.go @@ -57,11 +57,11 @@ func TestSimpleArrayExport(t *testing.T) { assert.False(t, test1IsReleased()) testarr := exportInt32Array() - arr, err := ImportCArrayWithType(&testarr, arrow.PrimitiveTypes.Int32) + arr, err := ImportCArrayWithType(testarr, arrow.PrimitiveTypes.Int32) assert.NoError(t, err) assert.False(t, test1IsReleased()) - assert.True(t, isReleased(&testarr)) + assert.True(t, isReleased(testarr)) arr.Release() runtime.GC() @@ -76,7 +76,7 @@ func TestSimpleArrayAndSchema(t *testing.T) { buflist := (*[2]unsafe.Pointer)(unsafe.Pointer(testarr.buffers)) origvals := (*[10]int32)(unsafe.Pointer(buflist[1])) - fld, arr, err := ImportCArray(&testarr, &sc) + fld, arr, err := ImportCArray(testarr, &sc) assert.NoError(t, err) assert.Equal(t, arrow.PrimitiveTypes.Int32, fld.Type) assert.EqualValues(t, 10, arr.Len()) diff --git a/go/arrow/cdata/cdata_test_framework.go b/go/arrow/cdata/cdata_test_framework.go index c9ccd74e0a2ee..68dbeb0d4b316 100644 --- a/go/arrow/cdata/cdata_test_framework.go +++ b/go/arrow/cdata/cdata_test_framework.go @@ -92,9 +92,9 @@ func getMetadataValues() ([]string, []string) { return []string{"", "bar"}, []string{"abcde"} } -func exportInt32Array() CArrowArray { - var arr CArrowArray - C.export_int32_array(C.get_data(), C.int64_t(10), &arr) +func exportInt32Array() *CArrowArray { + arr := C.get_test_arr() + C.export_int32_array(C.get_data(), C.int64_t(10), arr) return arr } diff --git a/go/parquet/internal/utils/min_max_arm64.go b/go/parquet/internal/utils/min_max_arm64.go new file mode 100644 index 0000000000000..f898d59953ec1 --- /dev/null +++ b/go/parquet/internal/utils/min_max_arm64.go @@ -0,0 +1,26 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// +build !noasm + +package utils + +func init() { + minmaxFuncs.i32 = int32MinMax + minmaxFuncs.ui32 = uint32MinMax + minmaxFuncs.i64 = int64MinMax + minmaxFuncs.ui64 = uint64MinMax +} diff --git a/go/parquet/internal/utils/min_max_s390x.go b/go/parquet/internal/utils/min_max_s390x.go new file mode 100644 index 0000000000000..f898d59953ec1 --- /dev/null +++ b/go/parquet/internal/utils/min_max_s390x.go @@ -0,0 +1,26 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// +build !noasm + +package utils + +func init() { + minmaxFuncs.i32 = int32MinMax + minmaxFuncs.ui32 = uint32MinMax + minmaxFuncs.i64 = int64MinMax + minmaxFuncs.ui64 = uint64MinMax +}