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

find: switch to deque for recent line storage #1910

Merged
merged 1 commit into from
Aug 11, 2020
Merged

find: switch to deque for recent line storage #1910

merged 1 commit into from
Aug 11, 2020

Conversation

dgw
Copy link
Member

@dgw dgw commented Jul 28, 2020

Description

Besides giving us roughly O(1) performance for appends instead of O(n), we also get automatic trimming of old entries.

Since I was already switching things up, I also eliminated the need to reverse the list (now deque) of lines before iterating through it when executing the find-and-replace operation. Lines are now stored in reverse chronological order.

Checklist

  • I have read CONTRIBUTING.md
  • I can and do license this contribution under the EFLv2
  • No issues are reported by make qa (runs make quality and make test)
  • I have tested the functionality of the things this change touches

Notes

I'm letting Travis do the QA checks, and will update that checkbox later.

@cottongin first suggested the idea, and expressed interest in helping test this change. I did do basic testing myself (including inspecting the data in memory with the ol' ipython plugin).

Probably should revisit a couple of my own plugins that also store recent messages like this and convert them over, since I just lazily copied find's logic.

Besides giving us roughly O(1) performance for appends instead of O(n),
we also get automatic trimming of old entries.

Since I was already switching things up, I also eliminated the need to
reverse the list (now deque) of lines before iterating through it when
executing the find-and-replace operation. Lines are now stored in
reverse chronological order.
@dgw dgw added this to the 7.1.0 milestone Jul 28, 2020
@dgw dgw requested a review from a team July 28, 2020 11:09
@cottongin
Copy link
Contributor

@cottongin expressed interest in helping test this change. I did do basic testing myself (including inspecting the data in memory with the ol' ipython plugin).

In my 18 hours or so of use I've yet to see any glaring issues. I like this a lot, though that could be because it was my idea. 😝

@dgw
Copy link
Member Author

dgw commented Jul 29, 2020

I like this a lot, though that could be because it was my idea. 😝

I edited the OP. Thought I'd credited you for the idea already, but my sleepy self forgot to do that.

Copy link
Contributor

@Exirel Exirel left a comment

Choose a reason for hiding this comment

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

🐑 it

@dgw dgw merged commit 2035fa9 into sopel-irc:master Aug 11, 2020
@dgw dgw deleted the find-deques branch August 11, 2020 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants