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

Don't throw out field data; use cache for citeproc #742

Merged
merged 2 commits into from
Mar 14, 2023
Merged

Don't throw out field data; use cache for citeproc #742

merged 2 commits into from
Mar 14, 2023

Conversation

bdarcus
Copy link
Contributor

@bdarcus bdarcus commented Mar 14, 2023

Can you take a look at this @andras-simonyi @andersjohansson?

It's a simple solution to the problem we've previously discussed, and an alternative to #710. I mostly stole code from org-cite-csl-activate!

But this should:

  1. fix citar-copy-reference slow? #702
  2. provide all the advantages of using the citar cache
  3. remove the potential limitation noted in the org-cite-csl-activate README.

The only problem I can potentially see is maybe it slows down initial loading of libraries a little, and uses a bit more memory. But I doubt it's a significant impact. And if it ever were, we could always add a defcustom so people could turn it off?

@andras-simonyi - not sure what you think about this, but one possibility is you can use the functions here instead, since you're already loading citar when available, and remove some code in org-cite-csl-activate in the process?

cc @benthamite

Ideally, we want citeproc to be able to use our cache. So we don't want
to throw out data it may need.

Close #741
@bdarcus bdarcus merged commit cb7d511 into main Mar 14, 2023
@bdarcus bdarcus deleted the parse-all branch March 14, 2023 20:57
@bdarcus
Copy link
Contributor Author

bdarcus commented Mar 14, 2023

I can't see any downside to this, so have merged it!

@andersjohansson
Copy link
Contributor

Got to this now and it seems to work well in my initial testing!

About performance:
For my 3.7 MiB json library file (exported from Zotero with BBT) I get very similar parsing times (4.5 s) when comparing before (97092df) and after (cb7d511) this change. I suppose this is reasonable since the parsing is the same only that some fields were discarded. memory-report gives 10 MiB versus 14 MiB for citar-cache--bibliographies. No big deal.

@bdarcus
Copy link
Contributor Author

bdarcus commented Mar 15, 2023

Great; thanks for the data!

BTW, you may also be interested in 82fdbae.

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.

citar-copy-reference slow?
2 participants