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

Pager mem leak #20

Merged
merged 6 commits into from
Jul 29, 2023
Merged

Pager mem leak #20

merged 6 commits into from
Jul 29, 2023

Conversation

KonstantinGasser
Copy link
Owner

in the previous function in the store/pager the func MoveDown would fill the pager's buffer up initially and once full truncate lines from the beginning in order to append the new lines at the end of the buffer. Lastly, MoveDow would recompute the bufferView if certain conditions where meet.

However, this function ignored that each time we do buffer = buffer[n:] we create a new slice still with the same backing array just a different window but with a changed capacity for the new slice. Than when appending the new lines with append(buffer, lines...) the capacity would not be enough and doubled in size. All this lead to a moving capacity..

The new function MovePosition shifts all items in the buffer to the left by N where N is the number of new lines which will be added to the buffer. As a result the capacity does not move and stays the same.

Even-though not noticeable in the benchmarks my assumption was that due to the static capacity runtime calls to growSlice and memMove would be reduced. Not completely sure why it's not reflecting in the benchmarks which where about the same (MovePosition took about 20ns longer but had about 200 fewer B/op).

Lastly, included in this Pull-Request is the change that MovePosition no longer builds the bufferView if the refresh-rate allows it to and the pager is not in the state paused. This functionality has been moved to the pager.String function.

current benchmarks for the new MovePosition func are:

BenchmarkMovePosition-12    	54673003	       216.2 ns/op	     206 B/op	       5 allocs/op

@KonstantinGasser KonstantinGasser merged commit 9b6d180 into main Jul 29, 2023
@KonstantinGasser KonstantinGasser deleted the pager_mem_leak branch July 29, 2023 20:37
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.

None yet

1 participant