From bd6c2117e850e2b259c0c3afb63c92de5f49b271 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Sun, 22 Dec 2019 08:11:24 -0500 Subject: [PATCH] fix analogous issue for history History couldn't be enabled by rehash if autoresize-window was nonzero. --- irc/history/history.go | 34 +++++++++++++++++++++++----------- irc/history/history_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 11 deletions(-) diff --git a/irc/history/history.go b/irc/history/history.go index 5f604c23..a70eb894 100644 --- a/irc/history/history.go +++ b/irc/history/history.go @@ -101,14 +101,7 @@ func NewHistoryBuffer(size int, window time.Duration) (result *Buffer) { } func (hist *Buffer) Initialize(size int, window time.Duration) { - initialSize := size - if window != 0 { - initialSize = initialAutoSize - if size < initialSize { - initialSize = size // min(initialAutoSize, size) - } - } - hist.buffer = make([]Item, initialSize) + hist.buffer = make([]Item, hist.initialSize(size, window)) hist.start = -1 hist.end = -1 hist.window = window @@ -118,6 +111,18 @@ func (hist *Buffer) Initialize(size int, window time.Duration) { hist.setEnabled(size) } +// compute the initial size for the buffer, taking into account autoresize +func (hist *Buffer) initialSize(size int, window time.Duration) (result int) { + result = size + if window != 0 { + result = initialAutoSize + if size < result { + result = size // min(initialAutoSize, size) + } + } + return +} + func (hist *Buffer) setEnabled(size int) { var enabled uint32 if size != 0 { @@ -339,12 +344,19 @@ func (list *Buffer) Resize(maximumSize int, window time.Duration) { list.maximumSize = maximumSize list.window = window - // if we're not autoresizing, we need to resize now; - // if we are autoresizing, we may need to shrink the buffer down to maximumSize, - // but we don't need to grow it now (we can just grow it on the next Add) + // three cases where we need to preemptively resize: + // (1) we are not autoresizing + // (2) the buffer is currently larger than maximumSize and needs to be shrunk + // (3) the buffer is currently smaller than the recommended initial size + // (including the case where it is currently disabled and needs to be enabled) // TODO make it possible to shrink the buffer so that it only contains `window` if window == 0 || maximumSize < len(list.buffer) { list.resize(maximumSize) + } else { + initialSize := list.initialSize(maximumSize, window) + if len(list.buffer) < initialSize { + list.resize(initialSize) + } } } diff --git a/irc/history/history_test.go b/irc/history/history_test.go index 10364437..e50cfb28 100644 --- a/irc/history/history_test.go +++ b/irc/history/history_test.go @@ -213,6 +213,35 @@ func TestAutoresize(t *testing.T) { assertEqual(atoi(items[len(items)-1].Nick), 271, t) } +// regression test for #702 +func TestEnabledByResize(t *testing.T) { + now := easyParse("2006-01-01 00:00:00Z") + // empty/disabled autoresizing buffer + buf := NewHistoryBuffer(0, time.Hour) + // enable the buffer as during a rehash + buf.Resize(128, time.Hour) + // add an item and test that it is stored and retrievable + buf.Add(autoItem(0, now)) + items := buf.Latest(0) + assertEqual(len(items), 1, t) + assertEqual(atoi(items[0].Nick), 0, t) +} + +func TestDisabledByResize(t *testing.T) { + now := easyParse("2006-01-01 00:00:00Z") + // enabled autoresizing buffer + buf := NewHistoryBuffer(128, time.Hour) + buf.Add(autoItem(0, now)) + items := buf.Latest(0) + assertEqual(len(items), 1, t) + assertEqual(atoi(items[0].Nick), 0, t) + + // disable as during a rehash, confirm that nothing can be retrieved + buf.Resize(0, time.Hour) + items = buf.Latest(0) + assertEqual(len(items), 0, t) +} + func TestRoundUp(t *testing.T) { assertEqual(roundUpToPowerOfTwo(2), 2, t) assertEqual(roundUpToPowerOfTwo(3), 4, t)