proposal: context: enable first class citizenship of third party implementations

[gobwas]

Hello there,

This proposal concerns a way to implement custom context.Context type, that is clever enough to cancel its children contexts during cancelation.

Suppose I have few goroutines which executes some tasks periodically. I need to provide two things to the tasks – first is the goroutine ID and second is a context-similar cancelation mechanism:

type WorkerContext struct {
    ID uint
    done chan struct{}
}

// WorkerContext implements context.Context.

func worker(id uint, tasks <-chan func(*WorkerContext)) {
    done := make(chan struct{})
    defer close(done)

    ctx := WorkerContext{
        ID: id,
        done: make(chan struct{}),
    }

    for task := range w.tasks {
        task(&ctx)
    }
}

go worker(1, tasks)
go worker(N, tasks)

Then, on the caller's side, the use of goroutines looks like this:

tasks <- func(wctx *WorkerContext) {
    // Do some worker related things with wctx.ID.

    ctx, cancel := context.WithTimeout(wctx, time.Second)
    defer cancel()

    doSomeWork(ctx)
}

Looking at the context.go code, if the given parent context is not of *cancelCtx type, it starts a separate goroutine to stick on parent.Done() channel and then propagates the cancelation.

The point is that for the performance sensitive applications starting of a separate goroutine could be an issue. And it could be avoided if WorkerContext could implement some interface to handle cancelation and track children properly. I mean something like this:

package context

type Canceler interface {
    Cancel(removeFromParent bool, err error)
}

type Tracker interface {
    AddChild(Canceler)
    RemoveChild(Canceler)
}

func propagateCancel(parent Context, child Canceler) {
	if parent.Done() == nil {
		return // parent is never canceled
	}
	if p, ok := parentCancelCtx(parent); ok { // p is now Tracker.
		if err := p.Err(); err != nil {
			// parent has already been canceled
			child.Cancel(false, err)
		} else {
			p.AddChild(child)
		}
	} else {
		go func() {
			select {
			case <-parent.Done():
				child.Cancel(false, parent.Err())
			case <-child.Done():
			}
		}()
	}
}

func parentCancelCtx(parent Context) (Tracker, bool) {
	for {
		switch c := parent.(type) {
                case Tracker:
			return c, true                
		case *timerCtx:
			return &c.cancelCtx, true
		case *valueCtx:
			parent = c.Context
		default:
			return nil, false
		}
	}
}

Also, with that changes we could even use custom contexts as children of created with context.WithCancel() and others:

type myCustonContext struct {} // Implements context.Canceler.

parent, cancel := context.WithCancel()
child := new(myCustomContext)
context.Bind(parent, child)

cancel() // Cancels child too.

Sergey.

[rsc]

/cc @Sajmani

I'm not convinced we want to expose the detail of the exact current Cancel signature.
In general waiting for cancellation requires a goroutine and that can be heavy (not just in this one case but in other uses too). We might want to think about that more general problem than this one short-circuit.

[Sajmani]

/cc @bcmills

Bryan has brought this up in the past, that we should make it possible for custom context implementations to avoid creating goroutines like the built-in implementation does.

In this example, it seems like the worker ID could be carried as a context value without a custom implementation. This would get you the efficiency you want at the cost of a runtime type assertion to extract the worker ID.

[rsc]

We know that there's no way to avoid new goroutines. Maybe that's important, maybe it's not. It's honestly unclear. Maybe things are fine as they are. The specific proposal here is probably not acceptable: we don't want to commit to exposing that specific API.

[gobwas]

@Sajmani seems like not only runtime type assertion, but also additional allocation to fill up interface{} header field with pointer to the, say, int ID?

[Sajmani]

There are some allocations involved in adding context values, yes. If that's causing significant overhead in your programs, could you please provide a profile? Then we can understand whether optimizations are necessary.

…

On Sun, Dec 23, 2018 at 8:12 AM Sergey Kamardin ***@***.***> wrote: @Sajmani <https://github.com/Sajmani> seems like not only runtime type assertion, but also additional allocation to fill up interface{} header field with pointer to the, say, int ID? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#28728 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AJSK3Sl1OgtOoXPjY86IQC-peii7PsH1ks5u74EkgaJpZM4YYsno> .

[rsc]

ping @gobwas, do you have any data to answer @Sajmani's question from Dec 23?

[gobwas]

Hello @rsc @Sajmani,

Sorry for silence.

It causing an overhead in general. Not any real world significant impact example yet. My point was to get rid of such overhead by doing a little in context’s API.

[rsc]

In general new API has a very high cost in constraining future implementations, what new users have to learn and so on. We don't add new API speculatively for overhead without real-world significant impact. There needs to be a real benefit to outweigh this real cost. Let's close this until we have a more pressing need.

[gobwas]

Okay, not a problem. By the way this may be implemented as an optional interface with just a type assertion, without adding cost of learning but adding flexibility of use.

[posener]

Proposal: context: Enable 1st class citizenship of 3rd party implementations

Problem statement

Third party context implementations suffer from performance degradation when being used alongside the standard library implementation, as every call to context.WithCancel, context.WithTimeout or context.WithDeadline results in invocation of a goroutine.

Above, the proposed solution is to expose Canceler and Tracker interfaces in the context package. It was declined because of the need to expose new interfaces, which are not sure to be needed, and to constrain the current context standard library implementation.

Proposal

I propose a compromise, in which, the current standard library implementation will use an unexported interfaces that contain exported functions. This solves the problem and addresses its disadvantages. I also propose a slightly different interface than the one in #28728. See draft implementation in posener/go@2641c4b:

// canceler is an optional interface for a context implementation for propagating
// cancellation. If a context implementation want to manage context
// cancellation it needs to cancel all its children when cancelled.
// This interface provides a way for a child to register itself in a parent context.
// When a context implements this interface and is cancelled, it promises to
// cancel all the registered contexts which have yet to unregister.
type canceler interface {
	// Register a new cancelable child for a context.
	Register(cancelable)
	// Unregister existing cancelable child.
	Unregister(cancelable)
}

The cancelable interface is a modified version of the current canceler interface. I found it awkward that the cancel method has the removeFromParent argument, as this responsibility should not be on the caller. I renamed the interface since whoever implements it enables being canceled and not canceling other contexts. In all the places it is used in the code it makes more sense now (at least to me).

// cancelable is an optional interface for a context implementation that can be
// canceled directly.
type cancelable interface {
	Cancel(err error)
	Done() <-chan struct{}
}

// child is an optional interface for a context implementation in which it returns its
// parent context. It is useful for context to expose this interface to prevent
// an extra goroutine when being called with a cancellable context conversion.
type child interface {
	// Return the parent context of a context.
	Parent() Context
}

This change is fully backward compatible.

Draft Implementation

See draft implementation in posener/go@2641c4b.

Since this proposal highly influences the design of the current context implementation, here is a draft implementation which shows the usage of these interfaces in the standard library.

Details

A nice thing that came up from this design is that now cancelCtx implements the cancelable and canceler interface, and valueCtx implements the child interface. The implementation of parentCancelCtx is much clearer now (it was renamed to lookupCanceler):

func lookupCanceler(parent Context) canceler {
	for {
		switch c := parent.(type) {
		case canceler:
			return c
		case child:
			parent = c.Parent()
		default:
			return &cancelerCtx{Context: parent}
		}
	}
}

From the code above, the cancelerCtx is a wrapper around Context that adds implementation of the canceler interface, by invoking the a goroutine in the Register function (Replaces the invocation in propagateCancel):

type cancelerCtx struct {
	Context
}

func (c *cancelerCtx) Register(child cancelable) {
	go func() {
		select {
		case <-c.Done():
			child.Cancel(c.Err())
		case <-child.Done():
		}
	}()
}

func (r *cancelerCtx) Unregister(child cancelable) {}

The valueCtx implements the child interface:

func (c *valueCtx) Parent() Context {
	return c.Context
}

And the cancelCtx implements the canceler and cancelable interfaces (very simple refactor of the existing logic).

Disadvantages

• There might be performance cost of adding interfaces usage.