gogctuner icon indicating copy to clipboard operation
gogctuner copied to clipboard

Algo values aren't a % / race condition

Open ashtonian opened this issue 3 years ago • 4 comments

I added some tests in #7 and some logging info and the tests logged:

2022/05/26 17:46:35 GC Tuner initialized. GOGC: 100 Target percentage: 70
2022/05/26 17:46:35 adjusting GOGC - from 100 to 582442.2222222222
2022/05/26 17:46:36 adjusting GOGC - from 100 to 43994.20740406999
2022/05/26 17:46:38 adjusting GOGC - from 582442 to 19957.129779857565

Seems like the values being calculated aren't a % and also like there is a bit of a race as one of the values didn't actually get set.

ashtonian avatar May 26 '22 22:05 ashtonian

I added some tests in #7 and some logging info and the tests logged:

2022/05/26 17:46:35 GC Tuner initialized. GOGC: 100 Target percentage: 70
2022/05/26 17:46:35 adjusting GOGC - from 100 to 582442.2222222222
2022/05/26 17:46:36 adjusting GOGC - from 100 to 43994.20740406999
2022/05/26 17:46:38 adjusting GOGC - from 582442 to 19957.129779857565

Seems like the values being calculated aren't a % and also like there is a bit of a race as one of the values didn't actually get set.

thx for your report, I'll dig for reason later

cch123 avatar May 27 '22 06:05 cch123

After learning more about gogc I think the number may make sense. I extracted the calc into a function and added some basic tests should be pretty easy to add some more test cases for the edges. Not sure how to test the getusage functions.. #9

ashtonian avatar May 31 '22 17:05 ashtonian

After learning more about gogc I think the number may make sense. I extracted the calc into a function and added some basic tests should be pretty easy to add some more test cases for the edges. Not sure how to test the getusage functions.. #9

getusage is direct function call into gopsutil and some proc sys read, maybe it's ok to skip these code for test?

cch123 avatar Jun 02 '22 04:06 cch123

Yea it would be nice to make sure they are returning values that make sense and are in the correct magnitude.

ashtonian avatar Jun 03 '22 20:06 ashtonian