Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Uber Go Style Guide

github.com

Post reply on HN
Print view View original

101–110 of 140 posts

Re: Uber Go Style Guide

#101

Post by _fbpt »

morelisp wrote: post #48
skybrian wrote: post #26

Earlier quoted context omitted.

Yes, that's not good, but it seems like in practice this doesn't come up too often in Go? More typically you're building a new slice containing the results of a query or other computation, and returning it without keeping a reference. Or instead of exposing an internal map, you have a Get method.

I have found this to be true of everything other than byte slices, where the result is some of the worst bugs I've had the displeasure of tracking down. Many Go libraries like to offer passing a byte slice to reuse as a destination. Many Go libraries take byte slices or structs containing them as arguments. A common result is "loop over some input, read it into the reusable slice, pass the slice to the next step". It…

In Python's Numpy library, `x[:]` also shares the same backing store. This bit me in the ass when I was learning Numpy.

Re: Uber Go Style Guide

#102

Post by xmprt »

ninkendo wrote: post #51
mytailorisrich wrote: post #41

Earlier quoted context omitted.

What else have they got to do, though? Uber have thousands of engineers to develop and maintain a taxi hailing app...

Work on their internal chat app, I guess? https://eng.uber.com/uchat/

That's built on top of Mattermost which is open source. They do build their own infra instead of using cloud though so that probably uses a big part of it.

Re: Uber Go Style Guide

#103

Post by jlokier »

This recommendation surprised me:

  if err := ioutil.WriteFile(name, data, 0644); err != nil {
   return err
  }
I've often seen guides for many languages that say don't use an assignment as an "if" condition. It may be a typo, and so is a source of errors, or it hides errors. Many compilers warn about it, and some people will consider it poor enough to refactor it out.

Of course it's not the assignment which is being tested in the Go code above. But it might as well be.

In Swift, Rust, Clojure and Perl, "if let" is common. ("if-let" in Clojure, "if my" in Perl). It's useful, and conforms well to the idea of limiting scope of the tested variable. I use it all the time.

So scope limiting in "if" is a good idea. But in those languages, they have the benefit of a clear syntax with a keyword, and it's a single assignment+test operator, very unlikely to be an accident due to a typo, or to hide an accident.

In contrast, I think the Go snippet looks error prone because the actual condition is all the way off the right. The assignment is obvious and the intention to test it can be presumed when skimming the code. But because the test is way off the right, at a horizontal position which will vary in different code, and at the end of a long line with a compound statement, I think it will be easy when skimming code to fail to notice if the condition is wrong due to a typo.

So I'm surprised Uber recommends this one.

Re: Uber Go Style Guide

#104

Post by Matthias247 »

jrockway wrote: post #65

Something that I don't see style guides addressing, but think they should, is how to cancel work when the caller no longer cares about the answer. (Consider an aborted RPC, or someone pressing the "stop" button in their browser.) A lot of people will do things like: func (s *Server) HandleFoo(ctx context.Context, in *input) status { ch This will block on the channel write even if the context becomes cancelled. Instea…

You are not crazy! However cancellation is an advanced topic. It requires some experience to get it right - and even to understand why it's required at all.

Most people will not see the need for cancellation - until they either need to implement a deadline timeout later on, or run into the problem that lots of outdated tasks take up their memory. Those things can come up years after the inital program had been written.

It's also not purely a Go thing. E.g. in Java thread interruptions are also barely handled correct. Or CancellationTokens in C#.

Re: Uber Go Style Guide

#105

Post by rictic »

weberc2 wrote: post #99
sanxiyn wrote: post #98

Earlier quoted context omitted.

How is it a fake problem? Uber is copying slices it doesn't need to copy, suffering significant performance penalty, because it is too error prone in Go. Rust completely prevents this error.

Ownership is different than mutability. Yes, immutability would prevent those errors.

The point is that ownership also prevents these errors. If there are never two simultaneous mutable references to something, then you can get the advantages of immutable data structures without paying the performance penalty.

Re: Uber Go Style Guide

#106

Post by Matthias247 »

palebluedot wrote: post #88

The preference of channel size being unbuffered or just 1 is interesting. That seems like something specific to a problem domain; for instance, in projects I am working on now, having a large buffered channel (1000s deep) is useful for worker queues of thousands of goroutines, that all read from a task feeder channel. This type of queuing seems go-idiomatic, and negates the need for additional synchronization. In thi…

Picking a queue size is a latency/throughput/determinism tradeoff.

Picking a higher queue size can increase the peak throughput. The queue will smoothen out peaks and valleys in the workload, and the other end of the queue will get rarer into situations where there is no work. If you know the duration of the "valleys" in your workload you can size your queue exactly to get over them.

However a too big queue size can easily lead to higher latencies. In extreme situations the work might be already outdated at the point of time it's processed by the receiver. And backpressure on the producer is less given.

Queue sizes > 1 can also mask concurrency issues (like deadlocks), which will then only show up rarely in production when the queue is fully exhausted. I guess that's one of the main reasons why they picked the 0/1 rule.

Re: Uber Go Style Guide

#107

Post by dpc_pw »

weberc2 wrote: post #93
dpc_pw wrote: post #56

Earlier quoted context omitted.

Exactly. Ownership/lifetimes exist in almost every mainstream PL (anywhere where you can have any sort of references + mutability). It's just people pretend it doesn't, and hope everything is going to be fine. And when you bring it in front of their consciousness they scream in panic, like it didn't exist before. Once you internalize Rust ownership rules, they are almost effortless and you see and obey them in any co…

Most languages have a GC so there really is no ownership to pretend away (if you like, everything is owned by the GC). Rust can be super cool without inventing fake problems for other languages.

I'm sorry, but that is a naive misconception. "everything is owned by the GC" is a sign of not understanding concept of ownership, BTW. GC is not an actor in the system, so does not participate in the ownership. A Java finalizers might kind of do stuff, but that's when all references are gone, so they have exclusive ownership.

Anyway... Languages with GC still do have ownership. It is just defaulting to be the most problematic case of "shared ownership" everywhere. Shared ownership is only worry-free if you don't ever modify data. Which is possible, but actually harder and more constraining to implement in a practical software (especially if you care about any performance). So then you either have to synchronize everything with mutexes, and figure out a way to not deadlock yourself on every step (also hard), or you have to implicitly track ownership and decide who can and when mutate the data, which is exactly what Rust would make you do, but this time you get no compiler help. The only reason people don't notice that is because they do it incorrectly and it works most of the time, so they are unaware.

There is no escape from it - one has to figure out ownership with a GC or without it, because GC has nothing to do with ownership. It only reclaims memory after all references are gone. That's it. That is actually very, very little improvement.

Re: Uber Go Style Guide

#108

Post by sansnomme »

sanxiyn wrote: post #4

"Copy Slices and Maps at Boundaries. Slices and maps contain pointers to the underlying data so be wary of scenarios when they need to be copied. Keep in mind that users can modify a map or slice you received as an argument if you store a reference to it. Similarly, be wary of user modifications to maps or slices exposing internal state." This could be used as an ad for Rust borrow checker, verbatim. You can't modify…

You can always write a borrow checker for Go and have the lifetime annotated in comments similar to how pre-3.7 Python type checkers worked.

In Rust, the borrow checker is too a separate static analysis stage and not directly tied to code gen. There are Rust compilers without the borrow checker.

Re: Uber Go Style Guide

#109

Post by TheDong »

sansnomme wrote: post #108
sanxiyn wrote: post #4

"Copy Slices and Maps at Boundaries. Slices and maps contain pointers to the underlying data so be wary of scenarios when they need to be copied. Keep in mind that users can modify a map or slice you received as an argument if you store a reference to it. Similarly, be wary of user modifications to maps or slices exposing internal state." This could be used as an ad for Rust borrow checker, verbatim. You can't modify…

You can always write a borrow checker for Go and have the lifetime annotated in comments similar to how pre-3.7 Python type checkers worked. In Rust, the borrow checker is too a separate static analysis stage and not directly tied to code gen. There are Rust compilers without the borrow checker.

If you wrote such syntax on top of Go, it would no longer be go, it would be a new language you created.

The go authors wouldn't accept it, the ecosystem wouldn't work with it, and it would be fighting an uphill battle rather than just using rust or building a new language.

> There are Rust compilers without the borrow checker

Then those compilers do not compile rust. They compile a bastardized version of rust whereby programs that are not valid rust programs can compile.

Also, can you name these non-borrowck-ing rust compilers? You say there's multiple, so you should be able to at least name one or two.

Re: Uber Go Style Guide

#110

Post by _caw »

fishywang wrote: post #94
jrockway wrote: post #73

Earlier quoted context omitted.

This is a very trivial example so doesn't dive into all the complexity. There is a lot of nuance here, like whether or not you really want to do an operation in the background in the first place. Background work does means that you lose the ability to apply backpressure to the calling system, and that will cause a lot of problems under load. Even if you do want to do the operation in the background, you still want to…

It does not need to be the background. You can check context cancellation without any goroutines by: select { default: case When there's no other channels to select with the context, just use that code block between lengthy operations to check context cancellation and return early. (If you are curious about how many extra time is wasted, it's easy to write a benchmark test for that. Last time I checked it was ~20ns w…

You may also simply write the following, which appears more elegant than the select.

  if ctx.Err() != nil {
    //then the context has expired or been cancelled
  }
Post reply on HN