Skip to content

proposal: Go2: rename "context" and (maybe) trim functionality #27987

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

Closed
deanveloper opened this issue Oct 3, 2018 · 8 comments
Closed

proposal: Go2: rename "context" and (maybe) trim functionality #27987

deanveloper opened this issue Oct 3, 2018 · 8 comments
Labels
FrozenDueToAge Proposal v2 An incompatible library change
Milestone

Comments

@deanveloper
Copy link

The problem with the name "context"

When I first started working with "context", I personally had a lot of trouble figuring out what the package even does. The name is not intuitive and suggests that it's purpose is for data transfer rather than cancelling tasks. While this is one of its purposes, after reading a number of articles, it really isn't good practice to use it. So really, "context" ends up just being used for cancellation of long-running tasks.

Why we should rename it

Instead, what we should have is a "cancel" package. The purpose is much more clear, and we can make names much more intuitive. I've personally made a quick mock-up of a "cancel" package, the implementation is MUCH simpler than "context", in fact, parent cancelers don't even need knowledge of their children. This may not be not important though as really implementation does not matter, as long as it is fast.

I find that the code is much more clear as to what it does, I've used it in a few internal projects at the company I work for. A lot of people there have never used Go before, here were my observations before/after the switch from "context" to "cancel":

  • Back when I used "context", it was hard to explain
    • The name was not intuitive to its task
    • This made explaining what it did very difficult
    • This also made reading code that used contexts difficult to read
  • Once switching to "cancel"
    • The name was intuitive, making it easy to explain
    • Even the "advanced" functionality (mainly functionality like parent Cancellers cancelling child Cancellers) was easy to explain because it had an intuitive name
    • People who were new to the language were able to use it easier
    • The trimmed functionality prevented misuse of the "data communication" aspect of "context"

Currently no public repositories use my "cancel" package (which makes sense, there's currently a stdlib package that does the exact same thing, and the stdlib package is most likely much more efficient).

Repetition

One of the design aspects in Go was to get rid of the whole Foo foo = new Foo() repetition, but with "context", we have func ExecContext(ctx context.Context). My package did not aim to fix this, as it has canc cancel.Canceller, but the repetition may be something to think about if we update "context" for Go2.

Other?

  • Of course we don't need to rename to "cancel", that's just the example I used.
  • Maybe the new package should have far fewer package-level functions?
    • ie canc, f = canc.Deadlined(deadline) instead of canc, f = cancel.WithDeadline(canc, deadline)
    • My API uses many package-level functions because I wanted it to be familiar to those who used "context" before.
  • Perhaps make "context" compatible with the new design? This is how I have designed my API, so any context.Context implements a cancel.Canceller.

Conclusion

The "context" package is hard to explain to new people because the name doesn't fit its purpose very well (at least for how it ended up being used). This proposal is to rename it to a more fitting name, and perhaps remove the "request-scoped values" aspect (as this part of the API is unintuitive, requires boilerplate for both setting and getting the value, and it is much more type-safe to just use function parameters anyway).

@gopherbot gopherbot added this to the Proposal milestone Oct 3, 2018
@deanveloper
Copy link
Author

Related: #27982

@ianlancetaylor
Copy link
Contributor

There really is a use for context values, though. See, for example, https://golang.org/pkg/runtime/pprof/#WithLabels and https://golang.org/pkg/runtime/pprof/#Do . These operations let a server associate labels with specific requests that will then appear in profiles, but it is only useful if you can pass the context along to all the goroutines doing work for that request. This lets you get request-specific profiles, which is extremely useful for analyzing server performance.

@augustoroman
Copy link
Contributor

@ianlancetaylor -- that's an interesting point. I think this speaks to the desire for runtime support of a built-in, goroutine-local storage -- at least if we separate the concepts of cancellation and context (which are correlated but not necessarily intrinsically intertwined). I feel like both of these concepts may warrant related (but independent) runtime support rather than making the convention of every single function in the language having context as a arg (e.g. io.Reader -> io.ReaderContext): if that's the case, then it may as well be part of the runtime.

@deanveloper
Copy link
Author

deanveloper commented Oct 3, 2018

There really is a use for context values, though.

at least if we separate the concepts of cancellation and context (which are correlated but not necessarily intrinsically intertwined)

I meant to mention that context values should have their own structure in the original post. I know uses for context values exist, but they are rarely used in everyday applications (in my experience) and are generally considered bad practice.

For instance, this blog post takes several paragraphs on how to properly use ctx.Value() and has several warnings
https://medium.com/@cep21/how-to-correctly-use-context-context-in-go-1-7-8f2c0fafdf39

This blog posts illustrates that context.Value is generally unsafe and should not be used, and offers a type-safe alternative to it
https://medium.com/@lestrrat/alternative-to-using-context-value-f2efe6bd2788

Another article which uses several paragraphs to illustrate that context.Value should only be used in very niche situations
https://www.calhoun.io/pitfalls-of-context-values-and-how-to-avoid-or-mitigate-them/

Also, note that NONE of these articles actually use context.WithValue and ctx.Value properly. As the documentation clearly states:

The provided key must be comparable and should not be of type string or any other built-in type to avoid collisions between packages using context. Users of WithValue should define their own types for keys. To avoid allocating when assigning to an interface{}, context keys often have concrete type struct{}. Alternatively, exported context key variables' static type should be a pointer or interface.

Not a single one of the articles (which, by the way, are the top 3 results in a google search for "uses for context values") illustrate using custom types for your context keys. The closest thing that comes to this is the second article, which just doesn't ever show the type of their key, but never explains anything about using a custom type for it.

@thejerf
Copy link

thejerf commented Oct 3, 2018

The problem with removing .Values() from the context value is that when you need it, there is no substitute. I find myself in the position of needing it and not having it (and making some stupid mostly-working global variable solution) often enough that removing it doesn't make much sense.

Also, while I myself do always define custom types as the documentation describes, the reality is that using string key isn't that big a deal. If you're trying to minimize context value usage anyhow (which I still agree is a good idea), it's unlikely that the two or three things you're using will conflict, and impossible for what may very well be the one thing you are using to conflict.

Also, I wonder if some of the reason you don't find the "context" name intuitive is precisely that you have never run into a situation where you need those values. It makes a lot more sense if you have a "context" value that is, for instance, storing the top-level value of your library and making it available to code that you have no other way to pass it through to. Part of the name "context" is precisely the ability to "set a context" by providing named values through the code. To be honest, for me it is often the case that I find the cancellation aspect of the context type superfluous to my needs!

@deanveloper
Copy link
Author

deanveloper commented Oct 3, 2018

The problem with removing .Values() from the context value is that when you need it, there is no substitute. I find myself in the position of needing it and not having it (and making some stupid mostly-working global variable solution) often enough that removing it doesn't make much sense.

Yeah, you're right. This is similar to what @ianlancetaylor was saying, there are real uses for .Values(). I meant to mention in my original proposal that there should still be a substitute for .Values(), that functionality just shouldn't be packaged along with cancellation.

Also, while I myself do always define custom types as the documentation describes, the reality is that using string key isn't that big a deal. If you're trying to minimize context value usage anyhow (which I still agree is a good idea), it's unlikely that the two or three things you're using will conflict, and impossible for what may very well be the one thing you are using to conflict.

I mainly just made a big deal out of the articles not using custom types because they're all describing how to use context, without even using it properly.

And while minimizing context value usage is one of the goals of this, the main goal still remains just to rename the "context" package for readability purposes. The package is used much more often for cancellation than it is for request-scoped variables, so when I'm describing it's main purpose to new people, it can be very confusing.

@ianlancetaylor
Copy link
Contributor

I think we agree that values need to be attached to contexts, so a simple rename to cancel may not be the best choice. In any case, I opened #28342 for a general discussion of context for Go 2. Closing this issue.

@deanveloper
Copy link
Author

Yeah my solution definitely did not do a good job at resolving all of the issues that people (including me) had with context. Good to get a discussion going though.

@golang golang locked and limited conversation to collaborators Oct 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge Proposal v2 An incompatible library change
Projects
None yet
Development

No branches or pull requests

5 participants