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

optimize window validation logic #576

Merged
merged 1 commit into from
Oct 24, 2024
Merged

Conversation

TanGentleman
Copy link
Contributor


name: pull request
about: submit changes to the project
title: "optimize window validation logic"
labels: 'enhancement'
assignees: 'TanGentleman'


description

Improve performance of is_valid_window() with several optimizations:

  • Add early returns for simple checks to avoid unnecessary processing
  • Cache lowercase string conversions of window name/title to prevent repeated allocations
  • Restructure logic flow to exit early on ignore list matches
  • Skip include list processing when empty

No changes in behavior, purely performance improvements by reducing redundant operations and string allocations.

related issue: #

type of change

  • bug fix
  • new feature
  • breaking change
  • documentation update
  • enhancement

how to test

checklist

  • i have read the CONTRIBUTING.md file
  • [X?] i have added the custom cursor AI prompt to my settings as mentioned in CONTRIBUTING.md and used to write this PR
  • my code follows the project's style guidelines
  • i have performed a self-review of my code
  • i have updated the documentation if necessary
  • my changes generate no new warnings
  • i have added tests that prove my fix is effective or that my feature works
  • all tests pass locally with my changes

additional notes

any other relevant information about the pr.

Improve performance of is_valid_window() with several optimizations:

- Add early returns for simple checks to avoid unnecessary processing
- Cache lowercase string conversions of window name/title to prevent repeated allocations 
- Restructure logic flow to exit early on ignore list matches
- Skip include list processing when empty

No changes in behavior, purely performance improvements by reducing redundant operations and string allocations.
Copy link

vercel bot commented Oct 24, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
screenpipe ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 24, 2024 1:14am

@TanGentleman
Copy link
Contributor Author

TanGentleman commented Oct 24, 2024

Ideally, I'd really appreciate exact string matching for App Name / Window name for ignored windows. I'm not sure the best way to pass this from the CLI to this function efficiently, but screenpipe seems to be recording 10s of thousands of windows that I have no trivial way of excluding!

Having too lax of a substring excludes things like a browser windows with that substring. I certainly want to save those, but it seems like if ignored, it overrides validity in include_list.

Perhaps an early return of True if the window is in include_list?

@louis030195
Copy link
Collaborator

Ideally, I'd really appreciate exact string matching for App Name / Window name for ignored windows. I'm not sure the best way to pass this from the CLI to this function efficiently, but screenpipe seems to be recording 10s of thousands of windows that I have no trivial way of excluding!

Having too lax of a substring excludes things like a browser windows with that substring. I certainly want to save those, but it seems like if ignored, it overrides validity in include_list.

Perhaps an early return of True if the window is in include_list?

hmm

i think some people (including me) like also the possibility to use a string contain for window include / exclude

but probably could make everything possible, in latest app release we added bunch of default in app settings to ignore useless windows but not yet added as default in cli

@louis030195 louis030195 merged commit f76d0d3 into mediar-ai:main Oct 24, 2024
2 checks passed
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.

2 participants