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

add caching of bib candidates #69

Closed
bdarcus opened this issue Apr 10, 2021 · 9 comments · Fixed by #71
Closed

add caching of bib candidates #69

bdarcus opened this issue Apr 10, 2021 · 9 comments · Fixed by #71
Labels
enhancement New feature or request
Milestone

Comments

@bdarcus
Copy link
Contributor

bdarcus commented Apr 10, 2021

Thanks for this nice package! I gave it a try and it worked smoothly for me.

I have a relatively large BibTeX file with over 1,000 entries (and 10,000 lines) and I have been using ivy-bibtex. The bibtex-ctions takes a noticeable time to read/load the BibTeX file every time, while ivy-bibtex takes a noticeable time only the first time (assuming that the BibTeX file has not been changed). I guess it is because ivy-bibtex somehow caches the BibTeX file that has been read the first time. Thus, IMHO, it would be great if bibtex-actions contains a similar caching mechanism.

Originally posted by @wenjie2wang in #68

@bdarcus bdarcus added the enhancement New feature or request label Apr 10, 2021
@bdarcus bdarcus added bug Something isn't working and removed enhancement New feature or request labels Apr 10, 2021
@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 10, 2021

@wenjie2wang can you please see here?

#70 (comment)

It should be working here.

@wenjie2wang
Copy link
Contributor

Thanks for your quick response! FYI, my bib file is available at https://gitlab.com/wenjie2wang/bibrary/-/raw/master/bib/index.bib.

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 10, 2021

OK, thanks.

I think I actually know what's going on. Will check in a bit.

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 10, 2021

I just loaded your file, and every time I load a command, there's a pause, but it's certainly less than a second.

What do you mean by "noticeable time"?

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 10, 2021

That said, I do think we've identified a performance bottleneck, that when fixed should address this.

@wenjie2wang
Copy link
Contributor

I just loaded your file, and every time I load a command, there's a pause, but it's certainly less than a second.

What do you mean by "noticeable time"?

Thanks, @bdarcus ! This is what I mean by "noticeable time". There is always a noticeable pause after M-x bibtex-actions-open. In contrast, M-x ivy-bibtex is able to respond instantly after the first run. I made a gif for your reference.
Peek 2021-04-10 12-23

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 10, 2021

OK, thanks much! That's what I see.

Not terrible, but I'd like performance to be consistently very good to excellent.

If you see the linked PR, I've figured out what causes this, but not yet why, or what can be done about it.

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 11, 2021

I've been digging into this (doing benchmarking and profiling), and I can't see an obvious way to improve this on my end without unnecessary complexity. But I do see that a simple change in bibtex-completion could address this.

The situation

We have two functions that generate this UI:

  1. bibtex-completion-candidates: this one is cached, and is fast
  2. bibtex-actions--get-candidates: this one is not cached, and is slower, because it transform 1 to do the display formatting

Because of how completing-read is designed, I need 2, though, because it doesn't have any notion of display transformers as in helm and ivy. So there is no distinction in completing-read between searching/filtering and display.

So part of the problem is intrinsic to completing-read.

The other part is that bibtex-completion-candidates is designed around the above feature that is in helm and ivy, but not in completing-read.

Proposed solution

Ideally, however, I'd be able to replace the candidate strings in bibtex-completion-candidates, with the output of bibtex-actions--get-candidates. That would allow all of the display data to be cached, and so address the pause you note.

@tmalsburg - would it be possible to add a hook to bibtex-completion (a la the sort of thing you see in org-mode) that would allow me to specify a different function to generate that string? Something like:

(add-hook 'bibtex-completion-candidates-hook #'bibtex-actions--format-candidate)

If yes, that would be a simple and elegant solution to this issue.

Edit: per below, however, I decided to add my own cache.

Conclusion

In the end, I don't think the slight delay is a deal breaker, but would ideally like to address it, without myself having to write my own caching code.

PS - I have tried different ways to speed up bibtex-actions--get-candidates, but none have an appreciable difference, so I don't think that's a promising path. It's basically fast enough, even for very large libraries, so long as we can activate caching on it.

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 11, 2021

I've opened a linked pr that adds a cache. A hook could still be useful, but if this works, it's a small enough addition that I'm happy to merge it.

@bdarcus bdarcus added the enhancement New feature or request label Apr 11, 2021
bdarcus added a commit that referenced this issue Apr 11, 2021
Refactor so that  'bibtex-actions-read' pulls candidates from a 
cache: 'bibtex-actions--candidates-cache'.

Add:

- 'bibtex-actions--candidates-cache' variable
- 'bibtex-actions-refresh' interactive command, to update cache
- README section on 'proactive reloading'

Refactor:

-  'bibtex-actions--get-candidates' renamed to `bibtex-actions--format-candidates`
-  'bibtex-actions--get-candidates' now grabs the candidates from the cache

fixes #69
@bdarcus bdarcus added this to the v1.0 milestone Apr 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants