- 
                Notifications
    You must be signed in to change notification settings 
- Fork 141
          perf: use rustc-hash for HashMap and HashSet
          #1298
        
          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
Conversation
Since we already depend on `rustc-hash` transiently, this doesn't add any more dependencies. As long as DOS attacks aren't a concern (which I don't think they are?), this should be free performance. In my (admittedly naive) testing, this really improved CPU usage in some cases, which is pretty nice to get for free.
| Are you able to use samply? Did you set  | 
| 
 I will check that out, thanks! 
 Yeah, I was using the  | 
| @mmstick I got it working! Just needed to give  Anyways, a quick benchmark opening a large folder (~2000 items) 
 
 | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regression testing passed:
Basic navigation
- Middle-click opens directory in a new tab (not focused).
- Open two scrollable tabs. Scroll one tab, then switch to the other tab; it should not have scrolled.
-  Hover over the top item in the folder, then scroll down until it's out of view (while still hovered). On scrolling back up (with the mouse in a different position), the item should not have the hover highlight.
- Not a regression.
 
-  Right-click an item in the sidebar. No visual change should occur with the rest of the items.
- Not a regression.
 
- Remove an item from the sidebar, then re-pin it.
File operations
- Right-click -> Create a new folder, then enter it.
- Right-click in the empty folder -> Create a new file.
- Files can be renamed.
- Files can be opened with non-default apps & browsing store for new apps works.
-  Normal right-click shows Move to trashoption.
-  Shift right-click, and right-click followed by Shift, both show Permanently deleteoption.- Shift + right-click broken (does nothing); right-click followed by shift works. Not a regression.
 
Advanced navigation & view settings
- Image and video thumbnails display in local folders.
- Gallery preview shows with Spacebar.
- Details pane shows with Ctrl+Spacebar.
- Zoom in/out and reset to default zoom work.
- Ctrl+1 and Ctrl+2 switch between list and icon view.
- Ctrl+H shows/hides hidden files.
- Directories can be sorted at top or inline.
- Settings -> Theme works.
- Settings -> Type to Search affects behavior as designed.
- Single-click to open setting takes effect.
- Sorting options work.
- Cutting, copying, and pasting files works.
- F5 reloads current directory.
- Left sidebar can be collapsed and expanded.
External filesystems
- Add a network drive (e.g. SFTP) and navigate into it.
- Plug in a USB drive; able to mount, browse, and eject.
Integrations
- Desktop icons display as expected
- Drag-and-drop into Firefox works
Since we already depend on
rustc-hashtransiently, this doesn't add any more dependencies. As long as DOS attacks aren't a concern (which I don't think they are?), this should be free performance.In my (admittedly naive) testing, this really improved CPU usage in some cases, which is pretty nice to get for free.
Also, on a separate note, I would like to work on improving the performance more, but right now I don't actually know how to benchmark properly (
cargo-flamegraphis returning nonsense for me?), so I've just been making random changes until performance improved. This is the first change I made that actually did anything meaningful 🫠