4 ms·

> What an amazing tool that can completely change function names when it converts from C to Go. How can one read the code of the benchmark, then switch into vi

> What an amazing tool that can completely change function names when it converts from C to Go. How can one read the code of the benchmark, then switch into virulent sarcasm mode without trying to understand the code? And seeing "+1" comments without any effort to understand is also disheartening. The blog post had a link about the Go helper functions the author used. It lands on https://github.com/akhenakh/goh3/blob/main/h3.go https://github.com/akhenakh/goh3/blob/main/h3.go This shows that the `FromGeo()` function used by the Go benchmark is a helper that calls transpiled functions. The benchmark code itself was of course not transpiled, so the sarcasm was unneeded and wrong. If anyone wants to dig in deeper, the C function `latLngToCell()` calls 2 functions, see https://github.com/uber/h3/blob/master/src/h3lib/lib/h3Index.c#L773 https://github.com/uber/h3/blob/master/src/h3lib/lib/h3Index... The Go function `XgeoToH3()` allocates a TLS then calls the same functions, see https://github.com/akhenakh/goh3/blob/main/ch3/a_linux_amd64.go#L9393 https://github.com/akhenakh/goh3/blob/main/ch3/a_linux_amd64... The C and Go code may not be strictly identical, but they seem pretty close. Enough to suppose the blog post was sincere. I believe the right thing to do is to ask the author why they used a custom rewrite of `latLngToCell()` in Go instead of calling the transpiled function.

whimsicalism 4y ago
Describing the GP as "virulent" is a silly over exaggeration.
bsdetector 4y ago
> The Go function `XgeoToH3()` allocates a TLS then calls the same functions And when author 'batches' the thread local storage he changes the code from thread-safe (C version) to not thread safe. The transpiled code is littered with TLS alloc and free calls to emulate stack variables that must be matched in order and that use the heap for temporary storage rather than the stack, with different cache access patterns that can very much affect tiny benchmarks. Contrary to what the author supposed about the performance, the fast Go version can't even be run over multiple threads. This is analogous to converting locks to NOPs and improving performance on a single-threaded microbenchmark, but doing so isn't an appropriate comparison even though the code may look identical except for one or two lines.
- jsmith45 4y ago
  > Contrary to what the author supposed about the performance, the fast Go version can't even be run over multiple threads. Since go deliberately fully hides the os threads, this TLS emulation would be per green thread (per go-routine). I don't see why the go code could not spin up say N goroutines, each one with one NewBatch() call to initialize the simulated TLS for that green thread, followed by repeatedly calling XgeoToH3() (which should be cleaning up its TLS usage to be back to 0 bytes used upon return). Needing to call NewBatch one per thread changes the API a little from the cgo API, but I don't think it is otherwise not thread safe.
- tedunangst 4y ago
  You'd allocate one TLS per thread, or have a reusable pool, if you wanted to run this multithreaded. Allocating and freeing a TLS per coord is not how the C code would run. If you created a new pthread per point in C that would be pretty slow too.
  - bsdetector 4y ago
    Yes, you can do this, but why didn't the code do this in the first place? Because you also want an API that's not annoying and difficult to use. It's a real drawback to this transpiler that it takes thread-safe, purely functional code and turns it into code with side effects that have to be carefully managed. Probably due to Go not having a fixed in place stack guarantee.
    - tedunangst 4y ago
      Well, it's not purely functional code. It calls malloc and free. And if you want a multithreaded malloc, you're going to need per thread state.
      - bsdetector 4y ago
        There are many parts of the code that are functional in C and have side effects in the transpiled version. Look ultimately what we have is a C version and cgo version that are roughly the same speed and a transpiled version that is 1/6th the speed - and the caller can be in any thread in any of those. Then there's a different API where the caller has to manage storage that's on par with the first two, but that's not the same thing. If you jump through these hoops you can be on par is a different claim from what the blog author made. Now it's possible that the wrapper functions could be made fast by storing the TLS object in a Go thread local storage so the API is the same, but the author didn't do this.

whimsicalism 4y ago

Describing the GP as "virulent" is a silly over exaggeration.

bsdetector 4y ago

> The Go function `XgeoToH3()` allocates a TLS then calls the same functions And when author 'batches' the thread local storage he changes the code from thread-safe (C version) to not thread safe. The transpiled code is littered with TLS alloc and free calls to emulate stack variables that must be matched in order and that use the heap for temporary storage rather than the stack, with different cache access patterns that can very much affect tiny benchmarks. Contrary to what the author supposed about the performance, the fast Go version can't even be run over multiple threads. This is analogous to converting locks to NOPs and improving performance on a single-threaded microbenchmark, but doing so isn't an appropriate comparison even though the code may look identical except for one or two lines.

jsmith45 4y ago
> Contrary to what the author supposed about the performance, the fast Go version can't even be run over multiple threads. Since go deliberately fully hides the os threads, this TLS emulation would be per green thread (per go-routine). I don't see why the go code could not spin up say N goroutines, each one with one NewBatch() call to initialize the simulated TLS for that green thread, followed by repeatedly calling XgeoToH3() (which should be cleaning up its TLS usage to be back to 0 bytes used upon return). Needing to call NewBatch one per thread changes the API a little from the cgo API, but I don't think it is otherwise not thread safe.
tedunangst 4y ago
You'd allocate one TLS per thread, or have a reusable pool, if you wanted to run this multithreaded. Allocating and freeing a TLS per coord is not how the C code would run. If you created a new pthread per point in C that would be pretty slow too.
- bsdetector 4y ago
  Yes, you can do this, but why didn't the code do this in the first place? Because you also want an API that's not annoying and difficult to use. It's a real drawback to this transpiler that it takes thread-safe, purely functional code and turns it into code with side effects that have to be carefully managed. Probably due to Go not having a fixed in place stack guarantee.
  - tedunangst 4y ago
    Well, it's not purely functional code. It calls malloc and free. And if you want a multithreaded malloc, you're going to need per thread state.
    - bsdetector 4y ago
      There are many parts of the code that are functional in C and have side effects in the transpiled version. Look ultimately what we have is a C version and cgo version that are roughly the same speed and a transpiled version that is 1/6th the speed - and the caller can be in any thread in any of those. Then there's a different API where the caller has to manage storage that's on par with the first two, but that's not the same thing. If you jump through these hoops you can be on par is a different claim from what the blog author made. Now it's possible that the wrapper functions could be made fast by storing the TLS object in a Go thread local storage so the API is the same, but the author didn't do this.