Skip to content

Commit

Permalink
executor: add heap object can move check for hash join v2 (#55200)
Browse files Browse the repository at this point in the history
ref #53127
  • Loading branch information
windtalker authored Aug 6, 2024
1 parent 1fb4019 commit 0e43ba0
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 3 deletions.
2 changes: 1 addition & 1 deletion pkg/executor/join/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ go_test(
],
embed = [":join"],
flaky = True,
shard_count = 48,
shard_count = 49,
deps = [
"//pkg/config",
"//pkg/domain",
Expand Down
2 changes: 1 addition & 1 deletion pkg/executor/join/hash_join_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func IsHashJoinV2Enabled() bool {
// sizeOfUintptr should always equal to sizeOfUnsafePointer, because according to golang's doc,
// a Pointer can be converted to an uintptr. Add this check here in case in the future go runtime
// change this
return enableHashJoinV2.Load() && sizeOfUintptr >= sizeOfUnsafePointer
return !heapObjectsCanMove() && enableHashJoinV2.Load() && sizeOfUintptr >= sizeOfUnsafePointer
}

// SetEnableHashJoinV2 enable/disable hash join v2
Expand Down
5 changes: 4 additions & 1 deletion pkg/executor/join/join_row_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ const sizeOfUintptr = int(unsafe.Sizeof(uintptr(0)))

var fakeAddrPlaceHolder = []byte{0, 0, 0, 0, 0, 0, 0, 0}

//go:linkname heapObjectsCanMove runtime.heapObjectsCanMove
func heapObjectsCanMove() bool

type rowTableSegment struct {
/*
The row storage used in hash join, the layout is
Expand Down Expand Up @@ -108,7 +111,7 @@ func setNextRowAddress(rowStart unsafe.Pointer, nextRowAddress unsafe.Pointer) {
// Save unsafe.Pointer into current Row header. Generally speaking it is unsafe or even illegal in go
// since after save the value of unsafe.Pointer into row header, it has no pointer semantics the value
// in the row header may become invalid after GC. It is ok to save unsafe.Pointer so far because
// 1. Go has a non-moving GC(https://tip.golang.org/doc/gc-guide), which means if the object is in heap, the address will not be changed after GC
// 1. the check of heapObjectsCanMove makes sure that if the object is in heap, the address will not be changed after GC
// 2. `rowStart` only points to a valid address in `rawData`. `rawData` is a slice in `rowTableSegment`, and it will be used by multiple goroutines,
// and its size will be runtime expanded, this kind of slice will always be allocated in heap
*(*unsafe.Pointer)(rowStart) = nextRowAddress
Expand Down
4 changes: 4 additions & 0 deletions pkg/executor/join/join_row_table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ import (
"github.com/stretchr/testify/require"
)

func TestHeapObjectCanMove(t *testing.T) {
require.Equal(t, false, heapObjectsCanMove())
}

func TestFixedOffsetInRowLayout(t *testing.T) {
require.Equal(t, 8, sizeOfNextPtr)
require.Equal(t, 8, sizeOfLengthField)
Expand Down

0 comments on commit 0e43ba0

Please sign in to comment.