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

Memoize result for a given input #19

Open
Ambrevar opened this issue May 23, 2023 · 4 comments
Open

Memoize result for a given input #19

Ambrevar opened this issue May 23, 2023 · 4 comments

Comments

@Ambrevar
Copy link
Member

A great optimization, I believe, would be to memoize the source update results depending on the input.

Example:

  • Type "foo", takes 10s to compute.
  • Type "bar", takes 15s to compute.
  • Type "foo" again.

With the current prompter, the last "foo" would take another 10s. With memoization, it would be instantaneous.

In terms of memory, I don't believe it's a big deal since prompters are short-lived objects.

@aartaka
Copy link
Contributor

aartaka commented May 23, 2023

This works for most cases, but not all. For example, it doesn't work for search engine prompts: we fetch the suggestions from the remove server, and these can change any moment. So memoizing search engine suggestions is quite misleading.

What I'd have is memoizing the results for the prompts that have a constant (= non-lambda) constructor. That should be pretty safe, because the only thing that can break in such a case is a pre/post-processor. But it's quite hard to conjure such a pre/post-processor that would break things, so we're safe.

Still, I'm not sure about this one.

@Ambrevar
Copy link
Member Author

The constructor is run only once during the prompter life-time ;)

Here I'm suggesting to memoize update: If the processors have no side effects, then it's a pure function, which makes memoization totally legit :)

Now what about side effects? As far as I'm aware, no Nyxt prompt has any.
But in any case, the solution is simple: just add a memoize-p prompter option (default to T), this way if the user wants to set impure filters, they disable memoization.

Thoughts?

@aartaka
Copy link
Contributor

aartaka commented May 24, 2023

Yeah, boolean toggle is the best solution here. But I'm somewhat cautious about setting it to T by default—seems like a potential foot-shooting behavior.

I mean, in case of Nyxt we can add a proxy source class with memoize-p set to T, but in the prompter itself I'd rather have it at NIL to not cause Heisenbugs downstream.

@Ambrevar
Copy link
Member Author

Fair enough, it's super easy to subclass prompter anyways :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants