Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime/cgo: parallel calls from C to Go causes high mutex lock contention on system with large CPUs #60961

Closed
cuonglm opened this issue Jun 23, 2023 · 3 comments
Assignees
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. Performance
Milestone

Comments

@cuonglm
Copy link
Member

cuonglm commented Jun 23, 2023

What version of Go are you using (go version)?

$ go version
go version devel go1.21-25e46693a1 Thu Jun 22 19:52:13 2023 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env

What did you do?

See https://groups.google.com/g/golang-nuts/c/rgKT2ptyeGQ/m/7i6tnfuuBAAJ.

What did you expect to see?

Program do not spend much time on lock/unlock.

What did you see instead?

Half of the time on lock contention.


Here's slightly version of original program:

package main

/*
#include <stdint.h>
#include <stdlib.h>
#include <string.h>

extern void c_call_go_get_value(uint8_t* key32, uint8_t *value32);

static inline void get_value(uint8_t* key32, uint8_t *value32) {
    uint8_t value[32];
    c_call_go_get_value(key32, value);
    memcpy(value32, value, 32);
}
*/
import "C"
import (
	"fmt"
	"runtime"
	"sync"
	"time"
	"unsafe"
)

func main() {
	numWorkers := runtime.NumCPU()
	fmt.Printf("numWorkers = %v\n", numWorkers)
	numTasks := 10_000_000
	tasks := make(chan struct{}, numTasks)
	for i := 0; i < numTasks; i++ {
		tasks <- struct{}{}
	}
	close(tasks)
	start := time.Now()
	var wg sync.WaitGroup
	wg.Add(numWorkers)
	for i := 0; i < numWorkers; i++ {
		go func() {
			defer wg.Done()
			for range tasks {
				_ = getValue([32]byte{})
			}
		}()
	}
	wg.Wait()
	fmt.Printf("took %vms\n", time.Since(start).Milliseconds())
}

func getValue(key [32]byte) []byte {
	key32 := (*C.uint8_t)(C.CBytes(key[:]))
	value32 := (*C.uint8_t)(C.malloc(32))
	C.get_value(key32, value32)
	ret := C.GoBytes(unsafe.Pointer(value32), 32)
	C.free(unsafe.Pointer(key32))
	C.free(unsafe.Pointer(value32))
	return ret
}

//export c_call_go_get_value
func c_call_go_get_value(key32 *C.uint8_t, value32 *C.uint8_t) {
	key := C.GoBytes(unsafe.Pointer(key32), 32)
	_ = key
	value := make([]byte, 32)
	for i := 0; i < len(value); i++ {
		*(*C.uint8_t)(unsafe.Pointer(uintptr(unsafe.Pointer(value32)) + uintptr(i))) = (C.uint8_t)(value[i])
	}
}
@cuonglm cuonglm added this to the Backlog milestone Jun 23, 2023
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jun 23, 2023
@cuonglm cuonglm added Performance and removed compiler/runtime Issues related to the Go compiler and/or runtime. labels Jun 23, 2023
@gopherbot
Copy link

Change https://go.dev/cl/505455 mentions this issue: runtime/cgo: reduce runtime init done check using atomic

@dmitshur
Copy link
Contributor

Thanks for reporting this and sending a CL.

Do you know if it's a performance regression was caused by changes in 1.21, or a pre-existing performance opportunity? That'll help determine if the fix can be considered for this release or should wait for 1.22 after the release freeze. Thanks.

@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 23, 2023
@cuonglm
Copy link
Member Author

cuonglm commented Jun 23, 2023

Thanks for reporting this and sending a CL.

Do you know if it's a performance regression was caused by changes in 1.21, or a pre-existing performance opportunity? That'll help determine if the fix can be considered for this release or should wait for 1.22 after the release freeze. Thanks.

This is not a regression, so should be for 1.22 cycle.

@dmitshur dmitshur modified the milestones: Backlog, Go1.22 Jun 23, 2023
@bcmills bcmills added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done. Performance
Projects
None yet
Development

No branches or pull requests

5 participants