code review 12699048: cmd/api: eliminate duplicate package import work

cmd/api: eliminate duplicate package import work

On my Mac, cuts the API checks from 15 seconds to 6 seconds.
Also clean up some tag confusion: go run list-of-files ignores tags.

[rsc, 2013-08-09 21:26:54]

Hello bradfitz (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go/

[bradfitz, 2013-08-09 21:41:29]

LGTM with or without below

https://codereview.appspot.com/12699048/diff/2001/src/cmd/api/goapi.go
File src/cmd/api/goapi.go (right):

https://codereview.appspot.com/12699048/diff/2001/src/cmd/api/goapi.go#newcode43
src/cmd/api/goapi.go:43: var ourTags = map[string]bool{}
add a comment.

// ourTags is the set of tags we care about over all contexts ...

https://codereview.appspot.com/12699048/diff/2001/src/cmd/api/goapi.go#newcod...
src/cmd/api/goapi.go:401: // It is a comma-separated string; the first part is
dir, the rest tags.
we have structs that support == nowadays.  just make a "tagKey" type and lose
the English?

https://codereview.appspot.com/12699048/diff/2001/src/cmd/api/goapi.go#newcod...
src/cmd/api/goapi.go:416: key := dir
var keyTags []string
for _, tag := range tags {
   if ctags[tag] {
       keyTags = append(keyTags, tag)
   }
}
strings.Sort(keyTags)  // is this guaranteed otherwise?
return tagKey{dir, strings.Join(keyTags, ",")}

https://codereview.appspot.com/12699048/diff/2001/src/run.bash
File src/run.bash (right):

https://codereview.appspot.com/12699048/diff/2001/src/run.bash#newcode185
src/run.bash:185: time go run $GOROOT/src/cmd/api/run.go
I would delete the time.  Noise.  And seems like debugging.

We can always time it by hand later (and I have often).

https://codereview.appspot.com/12699048/diff/2001/src/run.bash#newcode185
src/run.bash:185: time go run $GOROOT/src/cmd/api/run.go
also remove the --tags from run.bat.

[gri, 2013-08-09 21:46:42]

FYI

https://codereview.appspot.com/12699048/diff/2001/src/cmd/api/goapi.go
File src/cmd/api/goapi.go (right):

https://codereview.appspot.com/12699048/diff/2001/src/cmd/api/goapi.go#newcode43
src/cmd/api/goapi.go:43: var ourTags = map[string]bool{}
I see this being assigned to, but never used. dregg?

https://codereview.appspot.com/12699048/diff/2001/src/cmd/api/goapi.go#newcod...
src/cmd/api/goapi.go:395: var (
please add comment about the key

// maps tag-based key -> package

?

https://codereview.appspot.com/12699048/diff/2001/src/cmd/api/goapi.go#newcod...
src/cmd/api/goapi.go:397: pkgTags  = map[string][]string{}
same here

[rsc, 2013-08-09 22:38:06]

https://codereview.appspot.com/12699048/diff/2001/src/cmd/api/goapi.go
File src/cmd/api/goapi.go (right):

https://codereview.appspot.com/12699048/diff/2001/src/cmd/api/goapi.go#newcode43
src/cmd/api/goapi.go:43: var ourTags = map[string]bool{}
On 2013/08/09 21:46:42, gri wrote:
> I see this being assigned to, but never used. dregg?

Yes, gone.

https://codereview.appspot.com/12699048/diff/2001/src/cmd/api/goapi.go#newcod...
src/cmd/api/goapi.go:395: var (
On 2013/08/09 21:46:42, gri wrote:
> please add comment about the key
> 
> // maps tag-based key -> package
> 
> ?

Done.

https://codereview.appspot.com/12699048/diff/2001/src/cmd/api/goapi.go#newcod...
src/cmd/api/goapi.go:397: pkgTags  = map[string][]string{}
On 2013/08/09 21:46:42, gri wrote:
> same here

Done.

https://codereview.appspot.com/12699048/diff/2001/src/cmd/api/goapi.go#newcod...
src/cmd/api/goapi.go:401: // It is a comma-separated string; the first part is
dir, the rest tags.
On 2013/08/09 21:41:29, bradfitz wrote:
> we have structs that support == nowadays.  just make a "tagKey" type and lose
> the English?

I'd still need the comma-separated key list since I have a variable number of
strings to use in the key. Left as is.

https://codereview.appspot.com/12699048/diff/2001/src/cmd/api/goapi.go#newcod...
src/cmd/api/goapi.go:416: key := dir
On 2013/08/09 21:41:29, bradfitz wrote:
> var keyTags []string
> for _, tag := range tags {
>    if ctags[tag] {
>        keyTags = append(keyTags, tag)
>    }
> }
> strings.Sort(keyTags)  // is this guaranteed otherwise?
> return tagKey{dir, strings.Join(keyTags, ",")}

The AllTags list is sorted, and the tags list came from AllTags. Added a
comment.

https://codereview.appspot.com/12699048/diff/2001/src/run.bash
File src/run.bash (right):

https://codereview.appspot.com/12699048/diff/2001/src/run.bash#newcode185
src/run.bash:185: time go run $GOROOT/src/cmd/api/run.go
On 2013/08/09 21:41:29, bradfitz wrote:
> I would delete the time.  Noise.  And seems like debugging.
> 
> We can always time it by hand later (and I have often).

We time everything else. I want to time this too. This is what I always feel
like I'm waiting for (perhaps unfairly, because it is at the end of the build),
and I want to see (every time) how it compares to the other things.

[rsc, 2013-08-09 22:44:04]

*** Submitted as https://code.google.com/p/go/source/detail?r=9e198c0d7d40 ***

cmd/api: eliminate duplicate package import work

On my Mac, cuts the API checks from 15 seconds to 6 seconds.
Also clean up some tag confusion: go run list-of-files ignores tags.

R=bradfitz, gri
CC=golang-dev
https://codereview.appspot.com/12699048