Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autotuning low and high bins' limits. #13

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Conversation

gallir
Copy link

@gallir gallir commented Jun 3, 2017

It solves the problem with buffers larger than 2^25 bits (32MB),
the histogram (calls) is skewed to the right and the performance
will be very if the percentile 95 is reached at the last bin.

I suffered this issue in our DOTW.com platform. The logs below are tests in one of our servers in production with actual traffic and load (I just changed temporarily the calibrateCallsThreshold to 12000 to get the logs faster).


2017/06/03 18:08:56 Bins [{0 64} {0 128} {12001 256} {1402 512} {0 1024} {0 2048} {479 4096} {2717 8192} {5050 16384} {7222 32768} {6681 65536} {5115 131072} {3805 262144} {2872 524288} {1426 1048576} {771 2097152} {465 4194304} {245 8388608} {184 16777216} {161 33554432}]

2017/06/03 18:08:56 Changed bins to 7 128

2017/06/03 18:24:46 Bins [{0 128} {12001 256} {1292 512} {0 1024} {0 2048} {443 4096} {2607 8192} {4972 16384} {7417 32768} {6770 65536} {4932 131072} {3686 262144} {2675 524288} {1304 1048576} {758 2097152} {469 4194304} {306 8388608} {171 16777216} {116 33554432} {47 67108864}]

2017/06/03 18:24:46 Changed bins to 8 256

2017/06/03 18:41:02 Bins [{12001 256} {1234 512} {0 1024} {0 2048} {531 4096} {2767 8192} {5197 16384} {7408 32768} {6625 65536} {4952 131072} {3602 262144} {2510 524288} {1298 1048576} {807 2097152} {398 4194304} {294 8388608} {208 16777216} {114 33554432} {44 67108864} {3 134217728}]

2017/06/03 18:57:48 Bins [{12001 256} {1281 512} {0 1024} {0 2048} {462 4096} {2835 8192} {4799 16384} {6801 32768} {6339 65536} {4846 131072} {3512 262144} {2703 524288} {1158 1048576} {702 2097152} {373 4194304} {226 8388608} {189 16777216} {92 33554432} {37 67108864} {7 134217728}]

gallir added 7 commits June 3, 2017 20:45
It solves the problem with buffers larger than 2^25 bits (32MB),
the histogram (calls) is skewed to the right and the performance
will be very if the percentile 95 is reached at the last bin.
Copy link

@erikdubbelboer erikdubbelboer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand your problem. If I look at your first log line I see only 161 allocations of 32MB or more?

}
return &ByteBuffer{
B: make([]byte, 0, atomic.LoadUint64(&p.defaultSize)),
}
}

// GetLen returns a buufer with its

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buffer

// in order to minimize GC overhead.
func GetLen(s int) *ByteBuffer { return defaultPool.GetLen(s) }

// GetLen return a buufer with its

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buffer

}

func init() {
log.Println("Using github.com/gallir/shortlivedpool instead of sync.pool")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't do this. It's super annoying if there is no way for people to disable this. Seeing as it's in init, packages that use this package don't have the possibility to call log.SetOutput before this.

import (
"log"

"github.com/gallir/shortlivedpool"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't recommend using shortlivedpool seeing as it stack allocates for every operation and has data race issues.

@@ -3,7 +3,7 @@ package bytebufferpool_test
import (
"fmt"

"github.com/valyala/bytebufferpool"
"github.com/gallir/bytebufferpool"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert this

@@ -1,3 +1,3 @@
module github.com/valyala/bytebufferpool
module github.com/gallir/bytebufferpool

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert this

@gaby
Copy link

gaby commented Apr 3, 2024

@gallir Can you update your PR? Those two lines point to your fork.

@erikdubbelboer Any chance of adding this?

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