Actually I want to create a benchmark which exactly highlights problems in scheduler/channels/etc. For that purpose I can't use your code
because (1) it rolls out own centralized scheduling via own channel, (2) parallel for will cause bottleneck on memory throughput so it can't potentially scale well.
My benchmarks results were interesting. The Go code was 2X faster than equivalent C code using pthreads, and 2X faster than a naive Go implementation like John's.
On Apr 14, 10:57 am, Steven <stev...@gmail.com> wrote:
> 4) You have more brackets than necessary. You don't need the brackets
> declaring Threshhold. You don't need the brackets in the if statements.
Running gofmt -w on the program would cleanup the program.
This looks good to me. I just have a few style notes.
1) Functions you want to be visible outside your package need to begin them with a capital letter. You seem to be following the "capitalize types, don't capitalize functions" model from other languages. This doesn't work for Go.
2) While you're changing models to accommodate this, I'll mention that most Go code I've seen follows CamelCaps.
3) Factory functions are normally called MakeMatrix (or NewMatrix if you're returning a pointer) rather than the other way around. If your package is called "matrix", you can just call it Make, since it'll be used as matrix.Make.
4) You have more brackets than necessary. You don't need the brackets declaring Threshhold. You don't need the brackets in the if statements.
5) It is idiomatic, when you have > 2 cases to choose between, to use a switch statement rather than if...else if...else. This helps make your logic for each case stand out above the syntax of the control structure.
6) You could make the signature of mathmult_pimpl:func matmult_pimpl (sync chan int, A, B, C Matrix, i0, i1, j0, j1, k0, k1 int)
A lot of this is of course subjective, but I thought I'd point them out :)
I did some analysis as part of a homework assignment recently. Here's my implementation:I think it's exactly the same as yours.
My benchmarks results were interesting. The Go code was 2X faster than equivalent C code using pthreads, and 2X faster than a naive Go implementation like John's.
I think your implementation is great. Who cares if it is "Go-ish" when it is fast and cache-optimized?
Yes. And, No :). I brought it up more because you were capitalizing
some things and not others based on a pattern that doesn't make sense
in Go. If you wanted to put your Matrix in a library, you would have
to make the change. In your main package it's unnecessary. I generally
follow the visible/non-visible approach in main anyways, because it
means less work if I decide to reorganize my source.
> 2) While you're changing models to accommodate this, I'll mention that most Go code I've seen follows CamelCaps.
> Hmmm.... I am looking at /test/bench sources and basically all code uses lowercase functions and variables...
That's because testing code isn't providing an API, and follows a
different naming convention for use by gotest.
> 3) Factory functions are normally called MakeMatrix (or NewMatrix if you're returning a pointer) rather than the other way around. If your package is called "matrix", you can just call it Make, since it'll be used as matrix.Make.
> It seems to contradict with "make" functions that create arrays and channels...Btw, the package is "main", it's single source benchmark.
NewT is definitely preferred and more common for most types. You see
MakeT sometimes, but rarely. Sometimes, if I'm doing extra work for a
type I'd normally make, I use MakeT. And I was just pointing out the
possibility of using the package name as part of the name for future
reference.
> 4) You have more brackets than necessary. You don't need the brackets declaring Threshhold. You don't need the brackets in the if statements.
> Is it a bad taste in Go? You know I still feel more comfortable with if (...) :)
To each their own. However, gofmt, I'm pretty sure, will remove the
brackets, and it's the definitive style for the Go language. I think
it's just a matter of getting used to it. I generally appreciate not
having the bracket clutter.
> 5) It is idiomatic, when you have > 2 cases to choose between, to use a switch statement rather than if...else if...else. This helps make your logic for each case stand out above the syntax of the control structure.
> Yeah, but it makes code inconsistent... and that "I've added third case so I need to rewrite it to switch... oh, no, I do not need that third case, I need to rewrite it back... ah, I need another third case..." :)
I must say that I've never run into this problem. I generally know
whether it's going to be a simple if or if...else upfront, and if it's
a bunch of options, I go with the switch.
> 6) You could make the signature of mathmult_pimpl:
> func matmult_pimpl (sync chan int, A, B, C Matrix, i0, i1, j0, j1, k0, k1 int)
>
>
>
> Aha!Actually after adoption from C code I had (... i int, j int, k, int ...), and I was stumbled by how it compiles and why I get errors like "incorrect number of arguments", now I understand it was (... i int, j int, k int, <unnamed> int ...) :)
I don't think it's a good idea, because it will produce very few very coarse grained and uneven tasks.
Btw, here are my performance results:And here are my current sources://why don't we split here, too?//one answer - at this point we've already got way more goroutines than procsThese 2 partitions can't run in parallel, otherwise they will produce data races.
John,
There are two forces at work in the parallel implemention Dmitriy and I shared. The first is parallelization to multiple cores. The second is partitioning the matrix to optimize cache hits. If you limit the number of branches in the algorithm based on the number of procs, you end up with big partitions that don't fit in cache well. If you use too many goroutines, the overhead of creating each will be substantial.
The solution is to recurse as much as before but only launch goroutines the first few levels down. Pass in an extra depth parameter and switch from the parallel version to the serial version after some max depth.
Ryanne
That's all well and good, but if you ever reach the case for splitting on k, i and j are already below threshold. The only change that happens is the for loops are separated.
--
You received this message because you are subscribed to a topic in the Google Groups "golang-nuts" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/golang-nuts/RVX-BhHcugM/unsubscribe.
To unsubscribe from this group and all its topics, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/32e55537-d2d0-4e9a-8425-c85ad6a790a1n%40googlegroups.com.
Blast from the past so it's hard to be sure, but I think that was how many rows or columns to pick for parallel sub matrices to multiply.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/8fa9bb79-938a-4f89-a0c9-792776151a6en%40googlegroups.com.