You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "zeroshade (via GitHub)" <gi...@apache.org> on 2025/04/28 20:51:45 UTC
[PR] chore(arrow): remove most lock copies [arrow-go]
zeroshade opened a new pull request, #362:
URL: https://github.com/apache/arrow-go/pull/362
### Rationale for this change
#326 moved to using `atomic.Int64{]` for the refcount, as a result we found many places that we were copying the value. For linting reasons we should remove those.
### What changes are included in this PR?
Utilizing generics to clean up the dictionary array code significantly, along with removing most of the cases where we were copying the refcount which contains a lock.
### Are these changes tested?
Yes, the existing unit tests cover it all.
### Are there any user-facing changes?
Nope.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore(arrow): remove most lock copies [arrow-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade merged PR #362:
URL: https://github.com/apache/arrow-go/pull/362
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore(arrow): remove most lock copies [arrow-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #362:
URL: https://github.com/apache/arrow-go/pull/362#discussion_r2064982928
##########
arrow/array/dictionary.go:
##########
@@ -1402,143 +1107,178 @@ func (b *BinaryDictionaryBuilder) ValueStr(i int) string {
return string(b.Value(i))
}
-type FixedSizeBinaryDictionaryBuilder struct {
- dictionaryBuilder
- byteWidth int
-}
-
-func (b *FixedSizeBinaryDictionaryBuilder) Append(v []byte) error {
- return b.appendValue(v[:b.byteWidth])
-}
-
-func (b *FixedSizeBinaryDictionaryBuilder) InsertDictValues(arr *FixedSizeBinary) (err error) {
- var (
- beg = arr.array.data.offset * b.byteWidth
- end = (arr.array.data.offset + arr.data.length) * b.byteWidth
- )
- data := arr.valueBytes[beg:end]
- for len(data) > 0 {
- if err = b.insertDictValue(data[:b.byteWidth]); err != nil {
- break
- }
- data = data[b.byteWidth:]
- }
- return
+type fsbType interface {
+ arrow.DayTimeInterval | arrow.MonthDayNanoInterval |
+ decimal.Decimal32 | decimal.Decimal64 | decimal.Decimal128 | decimal.Decimal256
}
-type Decimal32DictionaryBuilder struct {
+type fixedSizeDictionaryBuilder[T fsbType] struct {
dictionaryBuilder
+ byteWidth int
}
-func (b *Decimal32DictionaryBuilder) Append(v decimal.Decimal32) error {
- return b.appendValue((*(*[arrow.Decimal32SizeBytes]byte)(unsafe.Pointer(&v)))[:])
-}
-
-func (b *Decimal32DictionaryBuilder) InsertDictValues(arr *Decimal32) (err error) {
- data := arrow.Decimal32Traits.CastToBytes(arr.values)
- for len(data) > 0 {
- if err = b.insertDictValue(data[:arrow.Decimal32SizeBytes]); err != nil {
- break
- }
- data = data[arrow.Decimal32SizeBytes:]
- }
- return
-}
-
-type Decimal64DictionaryBuilder struct {
- dictionaryBuilder
-}
-
-func (b *Decimal64DictionaryBuilder) Append(v decimal.Decimal64) error {
- return b.appendValue((*(*[arrow.Decimal64SizeBytes]byte)(unsafe.Pointer(&v)))[:])
-}
-
-func (b *Decimal64DictionaryBuilder) InsertDictValues(arr *Decimal64) (err error) {
- data := arrow.Decimal64Traits.CastToBytes(arr.values)
- for len(data) > 0 {
- if err = b.insertDictValue(data[:arrow.Decimal64SizeBytes]); err != nil {
- break
- }
- data = data[arrow.Decimal64SizeBytes:]
- }
- return
-}
-
-type Decimal128DictionaryBuilder struct {
- dictionaryBuilder
-}
-
-func (b *Decimal128DictionaryBuilder) Append(v decimal128.Num) error {
- return b.appendValue((*(*[arrow.Decimal128SizeBytes]byte)(unsafe.Pointer(&v)))[:])
-}
-
-func (b *Decimal128DictionaryBuilder) InsertDictValues(arr *Decimal128) (err error) {
- data := arrow.Decimal128Traits.CastToBytes(arr.values)
- for len(data) > 0 {
- if err = b.insertDictValue(data[:arrow.Decimal128SizeBytes]); err != nil {
- break
- }
- data = data[arrow.Decimal128SizeBytes:]
+func (b *fixedSizeDictionaryBuilder[T]) Append(v T) error {
+ if v, ok := any(v).([]byte); ok {
+ return b.appendBytes(v[:b.byteWidth])
}
- return
-}
-
-type Decimal256DictionaryBuilder struct {
- dictionaryBuilder
-}
-func (b *Decimal256DictionaryBuilder) Append(v decimal256.Num) error {
- return b.appendValue((*(*[arrow.Decimal256SizeBytes]byte)(unsafe.Pointer(&v)))[:])
+ sliceHdr := struct {
+ Addr *T
+ Len int
+ Cap int
+ }{&v, b.byteWidth, b.byteWidth}
+ slice := *(*[]byte)(unsafe.Pointer(&sliceHdr))
+ return b.appendValue(slice)
}
-func (b *Decimal256DictionaryBuilder) InsertDictValues(arr *Decimal256) (err error) {
- data := arrow.Decimal256Traits.CastToBytes(arr.values)
+func (b *fixedSizeDictionaryBuilder[T]) InsertDictValues(arr arrValues[T]) (err error) {
+ data := arrow.GetBytes(arr.Values())
for len(data) > 0 {
- if err = b.insertDictValue(data[:arrow.Decimal256SizeBytes]); err != nil {
+ if err = b.insertDictBytes(data[:b.byteWidth]); err != nil {
break
}
- data = data[arrow.Decimal256SizeBytes:]
+ data = data[b.byteWidth:]
}
return
}
-type MonthDayNanoDictionaryBuilder struct {
+type FixedSizeBinaryDictionaryBuilder struct {
dictionaryBuilder
+ byteWidth int
}
-func (b *MonthDayNanoDictionaryBuilder) Append(v arrow.MonthDayNanoInterval) error {
- return b.appendValue((*(*[arrow.MonthDayNanoIntervalSizeBytes]byte)(unsafe.Pointer(&v)))[:])
+func (b *FixedSizeBinaryDictionaryBuilder) Append(v []byte) error {
+ return b.appendValue(v[:b.byteWidth])
}
-func (b *MonthDayNanoDictionaryBuilder) InsertDictValues(arr *MonthDayNanoInterval) (err error) {
- data := arrow.MonthDayNanoIntervalTraits.CastToBytes(arr.values)
+func (b *FixedSizeBinaryDictionaryBuilder) InsertDictValues(arr *FixedSizeBinary) (err error) {
+ var (
+ beg = arr.array.data.offset * b.byteWidth
+ end = (arr.array.data.offset + arr.data.length) * b.byteWidth
+ )
+ data := arr.valueBytes[beg:end]
for len(data) > 0 {
- if err = b.insertDictValue(data[:arrow.MonthDayNanoIntervalSizeBytes]); err != nil {
+ if err = b.insertDictValue(data[:b.byteWidth]); err != nil {
break
}
- data = data[arrow.MonthDayNanoIntervalSizeBytes:]
+ data = data[b.byteWidth:]
}
return
}
-type DayTimeDictionaryBuilder struct {
- dictionaryBuilder
-}
-
-func (b *DayTimeDictionaryBuilder) Append(v arrow.DayTimeInterval) error {
- return b.appendValue((*(*[arrow.DayTimeIntervalSizeBytes]byte)(unsafe.Pointer(&v)))[:])
-}
-
-func (b *DayTimeDictionaryBuilder) InsertDictValues(arr *DayTimeInterval) (err error) {
- data := arrow.DayTimeIntervalTraits.CastToBytes(arr.values)
- for len(data) > 0 {
- if err = b.insertDictValue(data[:arrow.DayTimeIntervalSizeBytes]); err != nil {
- break
- }
- data = data[arrow.DayTimeIntervalSizeBytes:]
- }
- return
-}
+// type Decimal32DictionaryBuilder struct {
Review Comment:
Nope, oops. Thanks!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] chore(arrow): remove most lock copies [arrow-go]
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #362:
URL: https://github.com/apache/arrow-go/pull/362#discussion_r2064974232
##########
arrow/array/dictionary.go:
##########
@@ -1402,143 +1107,178 @@ func (b *BinaryDictionaryBuilder) ValueStr(i int) string {
return string(b.Value(i))
}
-type FixedSizeBinaryDictionaryBuilder struct {
- dictionaryBuilder
- byteWidth int
-}
-
-func (b *FixedSizeBinaryDictionaryBuilder) Append(v []byte) error {
- return b.appendValue(v[:b.byteWidth])
-}
-
-func (b *FixedSizeBinaryDictionaryBuilder) InsertDictValues(arr *FixedSizeBinary) (err error) {
- var (
- beg = arr.array.data.offset * b.byteWidth
- end = (arr.array.data.offset + arr.data.length) * b.byteWidth
- )
- data := arr.valueBytes[beg:end]
- for len(data) > 0 {
- if err = b.insertDictValue(data[:b.byteWidth]); err != nil {
- break
- }
- data = data[b.byteWidth:]
- }
- return
+type fsbType interface {
+ arrow.DayTimeInterval | arrow.MonthDayNanoInterval |
+ decimal.Decimal32 | decimal.Decimal64 | decimal.Decimal128 | decimal.Decimal256
}
-type Decimal32DictionaryBuilder struct {
+type fixedSizeDictionaryBuilder[T fsbType] struct {
dictionaryBuilder
+ byteWidth int
}
-func (b *Decimal32DictionaryBuilder) Append(v decimal.Decimal32) error {
- return b.appendValue((*(*[arrow.Decimal32SizeBytes]byte)(unsafe.Pointer(&v)))[:])
-}
-
-func (b *Decimal32DictionaryBuilder) InsertDictValues(arr *Decimal32) (err error) {
- data := arrow.Decimal32Traits.CastToBytes(arr.values)
- for len(data) > 0 {
- if err = b.insertDictValue(data[:arrow.Decimal32SizeBytes]); err != nil {
- break
- }
- data = data[arrow.Decimal32SizeBytes:]
- }
- return
-}
-
-type Decimal64DictionaryBuilder struct {
- dictionaryBuilder
-}
-
-func (b *Decimal64DictionaryBuilder) Append(v decimal.Decimal64) error {
- return b.appendValue((*(*[arrow.Decimal64SizeBytes]byte)(unsafe.Pointer(&v)))[:])
-}
-
-func (b *Decimal64DictionaryBuilder) InsertDictValues(arr *Decimal64) (err error) {
- data := arrow.Decimal64Traits.CastToBytes(arr.values)
- for len(data) > 0 {
- if err = b.insertDictValue(data[:arrow.Decimal64SizeBytes]); err != nil {
- break
- }
- data = data[arrow.Decimal64SizeBytes:]
- }
- return
-}
-
-type Decimal128DictionaryBuilder struct {
- dictionaryBuilder
-}
-
-func (b *Decimal128DictionaryBuilder) Append(v decimal128.Num) error {
- return b.appendValue((*(*[arrow.Decimal128SizeBytes]byte)(unsafe.Pointer(&v)))[:])
-}
-
-func (b *Decimal128DictionaryBuilder) InsertDictValues(arr *Decimal128) (err error) {
- data := arrow.Decimal128Traits.CastToBytes(arr.values)
- for len(data) > 0 {
- if err = b.insertDictValue(data[:arrow.Decimal128SizeBytes]); err != nil {
- break
- }
- data = data[arrow.Decimal128SizeBytes:]
+func (b *fixedSizeDictionaryBuilder[T]) Append(v T) error {
+ if v, ok := any(v).([]byte); ok {
+ return b.appendBytes(v[:b.byteWidth])
}
- return
-}
-
-type Decimal256DictionaryBuilder struct {
- dictionaryBuilder
-}
-func (b *Decimal256DictionaryBuilder) Append(v decimal256.Num) error {
- return b.appendValue((*(*[arrow.Decimal256SizeBytes]byte)(unsafe.Pointer(&v)))[:])
+ sliceHdr := struct {
+ Addr *T
+ Len int
+ Cap int
+ }{&v, b.byteWidth, b.byteWidth}
+ slice := *(*[]byte)(unsafe.Pointer(&sliceHdr))
+ return b.appendValue(slice)
}
-func (b *Decimal256DictionaryBuilder) InsertDictValues(arr *Decimal256) (err error) {
- data := arrow.Decimal256Traits.CastToBytes(arr.values)
+func (b *fixedSizeDictionaryBuilder[T]) InsertDictValues(arr arrValues[T]) (err error) {
+ data := arrow.GetBytes(arr.Values())
for len(data) > 0 {
- if err = b.insertDictValue(data[:arrow.Decimal256SizeBytes]); err != nil {
+ if err = b.insertDictBytes(data[:b.byteWidth]); err != nil {
break
}
- data = data[arrow.Decimal256SizeBytes:]
+ data = data[b.byteWidth:]
}
return
}
-type MonthDayNanoDictionaryBuilder struct {
+type FixedSizeBinaryDictionaryBuilder struct {
dictionaryBuilder
+ byteWidth int
}
-func (b *MonthDayNanoDictionaryBuilder) Append(v arrow.MonthDayNanoInterval) error {
- return b.appendValue((*(*[arrow.MonthDayNanoIntervalSizeBytes]byte)(unsafe.Pointer(&v)))[:])
+func (b *FixedSizeBinaryDictionaryBuilder) Append(v []byte) error {
+ return b.appendValue(v[:b.byteWidth])
}
-func (b *MonthDayNanoDictionaryBuilder) InsertDictValues(arr *MonthDayNanoInterval) (err error) {
- data := arrow.MonthDayNanoIntervalTraits.CastToBytes(arr.values)
+func (b *FixedSizeBinaryDictionaryBuilder) InsertDictValues(arr *FixedSizeBinary) (err error) {
+ var (
+ beg = arr.array.data.offset * b.byteWidth
+ end = (arr.array.data.offset + arr.data.length) * b.byteWidth
+ )
+ data := arr.valueBytes[beg:end]
for len(data) > 0 {
- if err = b.insertDictValue(data[:arrow.MonthDayNanoIntervalSizeBytes]); err != nil {
+ if err = b.insertDictValue(data[:b.byteWidth]); err != nil {
break
}
- data = data[arrow.MonthDayNanoIntervalSizeBytes:]
+ data = data[b.byteWidth:]
}
return
}
-type DayTimeDictionaryBuilder struct {
- dictionaryBuilder
-}
-
-func (b *DayTimeDictionaryBuilder) Append(v arrow.DayTimeInterval) error {
- return b.appendValue((*(*[arrow.DayTimeIntervalSizeBytes]byte)(unsafe.Pointer(&v)))[:])
-}
-
-func (b *DayTimeDictionaryBuilder) InsertDictValues(arr *DayTimeInterval) (err error) {
- data := arrow.DayTimeIntervalTraits.CastToBytes(arr.values)
- for len(data) > 0 {
- if err = b.insertDictValue(data[:arrow.DayTimeIntervalSizeBytes]); err != nil {
- break
- }
- data = data[arrow.DayTimeIntervalSizeBytes:]
- }
- return
-}
+// type Decimal32DictionaryBuilder struct {
Review Comment:
Did you mean to leave this code commented out?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org