Skip to content
This repository has been archived by the owner on Sep 6, 2022. It is now read-only.

Get Rid Of GoProcess pls #106

Closed
bonedaddy opened this issue Jan 22, 2020 · 1 comment
Closed

Get Rid Of GoProcess pls #106

bonedaddy opened this issue Jan 22, 2020 · 1 comment

Comments

@bonedaddy
Copy link

This repository might've been useful 5 years ago when it was first created, but context has gotten really good first-class support in Golang, and its entirely possible to do exactly what goprocess does, except better, and more efficient.

Generally I see goprocess being used in a lot of places where goroutines are created. This makes creating a goroutine a very non-lightweight operation which is the exact opposite. Creating a goroutine should be lightweight!

Consider what happens everytime the Go function is called from goprocess, and this is done a lot!!! (https://github.com/jbenet/goprocess/blob/7f9d9ed286badffcf2122cfeb383ec37daf92508/impl-mutex.go#L26).

	return &process{
		teardown: tf,
		closed:   make(chan struct{}),
		closing:  make(chan struct{}),
		waitfors: make(map[*processLink]struct{}),
		children: make(map[*processLink]struct{}),
	}

That means every single goroutine we spin up creates a fuck ton of allocation. And its done twice every time Go is called!

// This is because having the process you
func Go(f ProcessFunc) Process {
	// return GoChild(Background(), f)

	// we use two processes, one for communication, and
	// one for ensuring we wait on the function (unclosable from the outside).
	p := newProcess(nil)
	waitFor := newProcess(nil)
	p.WaitFor(waitFor) // prevent p from closing
	go func() {
		f(p)
		waitFor.Close() // allow p to close.
		p.Close()       // ensure p closes.
	}()
	return p
}

There's so many simpler and more idiomatic approaches like:

go func() {
select {
case <-ctx.Done():
default:
}
  // do stuff
}()

In some of the forks I have for libp2p, and ipfs libraries the profiling I do makes jbenet/goprocess
show up as a noticeable consumer of resources, and I encourage this to be done by libp2p developers because it's pretty clear goprocess needs to go.

@marten-seemann
Copy link
Contributor

goprocess was completely removed from the libp2p code base. This was not an easy change, see libp2p/go-libp2p#1193 and libp2p/go-libp2p#1266 for example.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants