code review 128680043: runtime: Loosen conditions in TestMemstat in an attempt...

runtime: Loosen conditions in TestMemstat in an attempt to fix the nacl/arm build.

[sanjay.m, 2014-08-25 05:23:21]

Hello golang-codereviews@googlegroups.com (cc: khr@golang.org),

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

[sanjay.m, 2014-08-25 05:34:49]

Worth noting that the build might not be broken, this feels like a fairly 
unlikely situation. The failure might not even reproduce.

Sanjay

[dvyukov, 2014-08-25 06:16:24]

https://codereview.appspot.com/128680043/diff/20001/src/pkg/runtime/malloc_te...
File src/pkg/runtime/malloc_test.go (right):

https://codereview.appspot.com/128680043/diff/20001/src/pkg/runtime/malloc_te...
src/pkg/runtime/malloc_test.go:44: if st.HeapIdle+st.HeapInuse !=
st.HeapSys-st.HeapReleased {
remove "-st.HeapReleased", released memory is accounted in HeapIdle

https://codereview.appspot.com/128680043/diff/20001/src/pkg/runtime/malloc_te...
src/pkg/runtime/malloc_test.go:45: t.Fatalf("Heap values do not add up: %+v",
*st)
please print the exact the values we are checking here, instead of whole *st

[sanjay.m, 2014-08-25 06:28:45]

Hello golang-codereviews@googlegroups.com, dvyukov@google.com (cc:
golang-codereviews@googlegroups.com, khr@golang.org),

Please take another look.

[dvyukov, 2014-08-25 06:59:26]

LGTM

[dvyukov, 2014-08-25 07:29:55]

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

runtime: Loosen conditions in TestMemstat in an attempt to fix the nacl/arm
build.

LGTM=dvyukov
R=golang-codereviews, dvyukov
CC=golang-codereviews, khr
https://codereview.appspot.com/128680043

Committer: Dmitriy Vyukov <dvyukov@google.com>

[gobot, 2014-08-25 07:39:57]

This CL appears to have broken the plan9-386-cnielsen builder.
See http://build.golang.org/log/ddc76b998ed4bc38df657faf16694985434910b9

[0intro, 2014-08-25 11:47:25]

It seems the test is failing on Plan 9 386 and amd64.

--- FAIL: TestMemStats (0.00s)
	malloc_test.go:45: HeapIdle(2351104) + HeapInuse(2891776) should be equal to
HeapSys(804265984), but isn't.
FAIL
FAIL	runtime	33.810s

--- FAIL: TestMemStats (0.00s)
	malloc_test.go:45: HeapIdle(3850240) + HeapInuse(5259264) should be equal to
HeapSys(670769152), but isn't.
FAIL
FAIL	runtime	28.206s

[dvyukov, 2014-08-25 12:01:34]

On 2014/08/25 11:47:25, 0intro wrote:
> It seems the test is failing on Plan 9 386 and amd64.
> 
> --- FAIL: TestMemStats (0.00s)
> 	malloc_test.go:45: HeapIdle(2351104) + HeapInuse(2891776) should be equal to
> HeapSys(804265984), but isn't.
> FAIL
> FAIL	runtime	33.810s
> 
> --- FAIL: TestMemStats (0.00s)
> 	malloc_test.go:45: HeapIdle(3850240) + HeapInuse(5259264) should be equal to
> HeapSys(670769152), but isn't.
> FAIL
> FAIL	runtime	28.206s


David, please add printing of the whole MemStats struct on failure and post the
output here.

[0intro, 2014-08-25 12:34:01]

MemStats

Alloc 46760
TotalAlloc 50688
Sys 807410812
Lookups 3
Mallocs 257
Frees 22

HeapAlloc 46760
HeapSys 805904384
HeapIdle 352256
HeapInuse 237568
HeapReleased 0
HeapObjects 235
HeapAlloc 46760

StackInuse 458752
StackSys 458752
MSpanInuse 2220
MSpanSys 16384
MCacheInuse 596
MCacheSys 16384
BuckHashSys 720156
GCSys 6608
OtherSys 288144

NextGC 52880
LastGC 1408969840699511111
PauseTotalNs 4067215
PauseNs 2513661 1553554
NumGC 2
EnableGC true
DebugGC false

-- 
David du Colombier

[0intro, 2014-08-25 12:35:17]

--- FAIL: TestMemStats (71.93s)
malloc_test.go:88: HeapIdle(352256) + HeapInuse(237568) should be
equal to HeapSys(805904384), but isn't.

-- 
David du Colombier

[dvyukov, 2014-08-25 12:41:17]

David, please test https://codereview.appspot.com/131190043

I have not tried to build it, so the code can require some fixes.





On Mon, Aug 25, 2014 at 4:35 PM, David du Colombier <0intro@gmail.com> wrote:
> --- FAIL: TestMemStats (71.93s)
> malloc_test.go:88: HeapIdle(352256) + HeapInuse(237568) should be
> equal to HeapSys(805904384), but isn't.
>
> --
> David du Colombier

[0intro, 2014-08-25 13:04:54]

Thanks. It looks better but we're still not there.

% go test -v -run TestMemStats
=== RUN TestMemStats
Alloc 46872
TotalAlloc 50800
Sys 2231420
Lookups 3
Mallocs 258
Frees 22

HeapAlloc 46872
HeapSys 724992
HeapIdle 352256
HeapInuse 237568
HeapReleased 0
HeapObjects 236

StackInuse 458752
StackSys 458752
MSpanInuse 2220
MSpanSys 16384
MCacheInuse 596
MCacheSys 16384
BuckHashSys 720156
GCSys 6608
OtherSys 288144

NextGC 53104
LastGC 1408971620079650117
PauseTotalNs 4211388
PauseNs 2506504 1704884
NumGC 2
EnableGC true
DebugGC false
--- FAIL: TestMemStats (34.73s)
malloc_test.go:88: HeapIdle(352256) + HeapInuse(237568) should be
equal to HeapSys(724992), but isn't.
FAIL
exit status: 'runtime.test 473678: 1'
FAIL runtime 35.040s

-- 
David du Colombier

[dvyukov, 2014-08-25 13:16:00]

try s/runtime·xadd64(&mstats.heap_sys, nbytes)/runtime·xadd64(stat,
nbytes)/ in SysMap

On Mon, Aug 25, 2014 at 5:04 PM, David du Colombier <0intro@gmail.com> wrote:
> Thanks. It looks better but we're still not there.
>
> % go test -v -run TestMemStats
> === RUN TestMemStats
> Alloc 46872
> TotalAlloc 50800
> Sys 2231420
> Lookups 3
> Mallocs 258
> Frees 22
>
> HeapAlloc 46872
> HeapSys 724992
> HeapIdle 352256
> HeapInuse 237568
> HeapReleased 0
> HeapObjects 236
>
> StackInuse 458752
> StackSys 458752
> MSpanInuse 2220
> MSpanSys 16384
> MCacheInuse 596
> MCacheSys 16384
> BuckHashSys 720156
> GCSys 6608
> OtherSys 288144
>
> NextGC 53104
> LastGC 1408971620079650117
> PauseTotalNs 4211388
> PauseNs 2506504 1704884
> NumGC 2
> EnableGC true
> DebugGC false
> --- FAIL: TestMemStats (34.73s)
> malloc_test.go:88: HeapIdle(352256) + HeapInuse(237568) should be
> equal to HeapSys(724992), but isn't.
> FAIL
> exit status: 'runtime.test 473678: 1'
> FAIL runtime 35.040s
>
> --
> David du Colombier

[0intro, 2014-08-25 13:28:17]

> try s/runtime·xadd64(&mstats.heap_sys, nbytes)/runtime·xadd64(stat,
> nbytes)/ in SysMap

Thanks. The test passes now.

-- 
David du Colombier