code review 4395042: container/heap: fix circular dependency in test

container/heap: fix circular dependency in test

Having the test be in the container/heap package yields a cycle
  container/heap (for the test)
  -> testing
  -> time
  -> container/heap (for timerHeap)

Occasionally the linker would get mixed up, resulting in a test panic
in a very weird place.

[dsymonds, 2011-04-13 00:41:27]

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

I'd like you to review this change to
https://go.googlecode.com/hg/

[r2, 2011-04-13 00:45:59]

i will express a slight reservation that this is pushing things in the wrong
direction.
i'd rather see the timing package get special time code, myself, although that
might be untenable.

container/heap is in the kernel??

i'll lose, but i wanted to express my dissatisfaction.

-rob

[dsymonds, 2011-04-13 00:56:23]

On Wed, Apr 13, 2011 at 10:45 AM, Rob 'Commander' Pike <r@google.com> wrote:

> i will express a slight reservation that this is pushing things in the wrong
direction.
> i'd rather see the timing package get special time code, myself, although that
might be untenable.

Yeah, that's a reasonable thing to do. This just fixes a problem that
occasionally causes all.bash to fail on my machine.
I showed it to rsc in person in March, and this was the workaround he
suggested (I think he suggested that the linker had a bug).


Dave.

[rsc, 2011-04-13 12:19:13]

I'd like to understand better what is going on.
The first layer of the mystery is explained by the fact
that if package P imports a package Q that doesn't have
any init code, P won't bother generating a call to Q.init,
so the run-time check for introduced cycles doesn't
see that dependency.  But I have no idea why the runtime
check does break once in a while, and only on David's
machine.

David, can you mail me a copy of a crashing 6.out?
Linux or OS X, I don't care.

Russ

[dsymonds, 2011-04-13 14:11:09]

On Wed, Apr 13, 2011 at 10:19 PM, Russ Cox <rsc@golang.org> wrote:

> David, can you mail me a copy of a crashing 6.out?
> Linux or OS X, I don't care.

Now, of course, I can't seem to reproduce it. At least you saw it in
person once and was as confused about it as me.

I'll grab a 6.out if I manage to reproduce it.


Dave.

[dsymonds, 2011-04-14 00:12:12]

On Thu, Apr 14, 2011 at 12:11 AM, David Symonds <dsymonds@golang.org> wrote:

> Now, of course, I can't seem to reproduce it. At least you saw it in
> person once and was as confused about it as me.
>
> I'll grab a 6.out if I manage to reproduce it.

Ooh, did it! I've got a container/heap test 6.out built from a
reasonably clean tree that reliably panics with "linker skew".
Russ, I'll send it to you off-list.


Dave.

[dsymonds, 2011-04-19 00:51:53]

FYI, this broke for me again just now.

[rsc, 2011-04-22 20:25:07]

David, do you by any chance have a
modified copy of package sort that
imports "reflect"?

[rsc, 2011-04-22 20:28:33]

On Tue, Apr 12, 2011 at 20:45, Rob 'Commander' Pike <r@google.com> wrote:
> i will express a slight reservation that this is pushing things in the wrong
direction.
> i'd rather see the timing package get special time code,
> myself, although that might be untenable.
>
> container/heap is in the kernel??

this is the testing kernel:

$ cd $GOROOT/src/pkg
$ make -n testing.install | awk '/^make -C/ {print $3}' | sort
bufio
bytes
container/heap
flag
fmt
io
io/ioutil
math
os
path/filepath
reflect
runtime
runtime/pprof
sort
strconv
strings
sync
sync/atomic
syscall
testing
time
unicode
utf16
utf8
$

on balance time should probably not depend on
container/heap, but i think that is mainly temporary.
i created issue 1731.

[rsc, 2011-04-22 20:28:39]

LGTM

[rsc, 2011-04-22 20:29:09]

*** Submitted as http://code.google.com/p/go/source/detail?r=5ca3b23b2f73 ***

container/heap: fix circular dependency in test

Having the test be in the container/heap package yields a cycle
  container/heap (for the test)
  -> testing
  -> time
  -> container/heap (for timerHeap)

Occasionally the linker would get mixed up, resulting in a test panic
in a very weird place.

R=rsc, r2
CC=golang-dev
http://codereview.appspot.com/4395042

Committer: Russ Cox <rsc@golang.org>

[dsymonds, 2011-04-23 00:21:17]

On Sat, Apr 23, 2011 at 6:25 AM, Russ Cox <rsc@golang.org> wrote:

> David, do you by any chance have a
> modified copy of package sort that
> imports "reflect"?

Yeah.