Skip to content

Conversation

@rihter007
Copy link
Contributor

Make multiple iterations checks to reduce sleep time in case no goroutines leaked

Make multiple iterations checks to reduce sleep time in case no goroutines leaked

Signed-off-by: Ilya <rihter007@inbox.ru>
@rihter007 rihter007 requested a review from mimir-d August 17, 2022 14:14
@mimir-d
Copy link
Member

mimir-d commented Aug 18, 2022

can you provide a before and after output for a leak detection? (on account of removing some of the field details)

@rihter007
Copy link
Contributor Author

can you provide a before and after output for a leak detection? (on account of removing some of the field details)

there are unit tests if you suspect a bug

@mimir-d
Copy link
Member

mimir-d commented Aug 18, 2022

i just wanted to see how that looks, since fields were removed. and how that makes the tests run faster

@rihter007
Copy link
Contributor Author

i just wanted to see how that looks, since fields were removed. and how that makes the tests run faster

Something like that:

leaked goroutines:
goroutine 22 [sync.Cond.Wait]:
sync.runtime_notifyListWait(0xc000090990, 0x1)
/usr/local/go/src/runtime/sema.go:513 +0x13d
sync.(*Cond).Wait(0x0)
/usr/local/go/src/sync/cond.go:56 +0x8c
github.com/linuxboot/contest/tests/common/goroutine_leak_check.Func1(0xc000090980)
/Users/ilyaarzamartsev/projects/contest/tests/common/goroutine_leak_check/goroutine_leak_check_test.go:19 +0x3d
created by github.com/linuxboot/contest/tests/common/goroutine_leak_check.TestLeaks
/Users/ilyaarzamartsev/projects/contest/tests/common/goroutine_leak_check/goroutine_leak_check_test.go:30 +0x9d

Actually same as before, previous implementation just made extra parsing that is not needed.

Why faster?

Instead of the logic:

Sleep(A_LOT)
checkLeakedGoroutines()

The new logic is

for tryIdx = 0; tryIdx < {
if checkLeakedGoroutines() == nil {
   break
}
Sleep(little)
}

So, new implementation should be faster...

@rihter007 rihter007 merged commit 5b8e421 into main Aug 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants