fasthttp icon indicating copy to clipboard operation
fasthttp copied to clipboard

[Suggestion] Use custom context for net.DialContext go1.18

Open moredure opened this issue 2 years ago • 3 comments

Hi, since fasthttp.Client isn't using context actively for cancelling Dial calls, for go1.18beta1 with unix systems, it may be more performant to use Dialer with custom made context to avoid spawning interruptor go-routine (and in cases when deadline firing earlier also context.WithDeadline internal goroutine time.AfterFunc) in case you can fully rely on netpoller to stop connect call instead of interruptor goroutine or context cancellation https://github.com/golang/go/blob/master/src/net/fd_unix.go#L117.

//go:build go1.18 && linux
// +build go1.18 linux
type deadlineCtx struct {
	deadline time.Time
}

func (m *deadlineCtx) Deadline() (deadline time.Time, ok bool) {
	return m.deadline, true
}

func (m *deadlineCtx) Done() <-chan struct{} {
	return nil // skip done in order to skip net/fd_unix.go interruptor goroutine creation
}

func (m *deadlineCtx) Err() error {
	return nil
}

func (m *deadlineCtx) Value(interface{}) interface{} {
	return nil
}

For more details you can check https://github.com/golang/go/issues/49023 In our use case (requests made with keepalive disabled), we cut in half number of application goroutines spawned . @erikdubbelboer what do you think, is it make sense or it should be user made custom Dial for fasthttp.Client/HostClient/PipelineClient?

Sample https://gist.github.com/moredure/a90e68e5e2e6a8e7fce67b7b1472ce51

moredure avatar Jan 08 '22 16:01 moredure

in case super strict deadlines not needed and you can fully rely on netpoller to stop connect call instead of interruptor goroutine

Do you have any experience with the difference in time here? I would expect the netpoller to be quite accurate as well with it's deadlines.

If the deadline of the netpoller is accurate enough as well I think it makes sense to just improve the current implementation instead of adding new methods.

But we would need some good tests to make sure it keeps working correctly in both Go versions.

erikdubbelboer avatar Jan 09 '22 03:01 erikdubbelboer

In fact I didn't see any worse difference in deadline accuracy with application running 15k goroutines each creating, read+writing and closing TCP connection per cycle with 32 logical cores, but maybe with higher number of CPUs it may affect it somehow, since, as I know, netpoller utilizes separate thread and not per core execution as plain goroutines, but it requires some more benchmarks to prove it. Also golang prior to go1.18 don't have ability to bypass interruptor goroutine with such a custom/special made context(with deadline but with nil Done channel) to net.DialContext to avoid interruptor goroutine creation, also works only with IP addresses, hostnames needed to be resolved prior. Actually in my case I don't modify my fork of fasthttp, but just created separate file with custom Dial function with build constraints, probably we can implement such a custom function with build constraints to go1.18 and linux/unix, but maybe it's not needed and we should just inform developers of possibility to reduce number of goroutines spawned that way, until Golang developers will develop another way to do it.

moredure avatar Jan 09 '22 12:01 moredure

I am working on https://github.com/golang/go/pull/50170 to make it work with windows as well

moredure avatar Jan 09 '22 12:01 moredure

I need my information

RhondaMorrison420 avatar Dec 01 '22 01:12 RhondaMorrison420