-
-
Notifications
You must be signed in to change notification settings - Fork 147
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
Faster SyncTeX by reusing scanner object #689
base: develop
Are you sure you want to change the base?
Conversation
zathura/synctex.c
Outdated
@@ -16,22 +17,74 @@ | |||
#include "adjustment.h" | |||
|
|||
#ifdef WITH_SYNCTEX | |||
synctex_scanner_p scanner = NULL; // the last scanner object | |||
time_t last_modification_time = 0; // the last modification time of the synctex file | |||
char* last_pdf_file_name = NULL; // the last output file name |
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.
This state would need to go to the zathura_t
struct. When the document is closed and reopened, also the scanners can be reopened.
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.
What do you mean by "the scanners can be reopened"?
I don't think reloading the synctex file early is a good idea consider that for a large document it can take up to a few seconds, which would make zathura look sluggish.
An alternative is to load it in a separate thread — that way CPU is consumed but otherwise nothing bad happens. What do you think?
zathura/zathura.h
Outdated
*/ | ||
struct { | ||
synctex_scanner_p scanner; | ||
time_t last_modification_time; // the last modification time of the synctex file |
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.
This doesn't need to be tracked. The idea would be that if the file is reloaded due to changes, the scanner is freed. The next time the scanner is used, it can be reopened.
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.
Makes sense.
Do you think it's worth considering the possibility that (somehow) only the synctex file is updated and not the PDF (e.g. the user surgically modify the synctex file somehow)?
What I think: in practice it will never happen, and the most common cases (e.g. the user delete the synctex file, a new synctex file appear because user move it from another directory) are gracefully handled.
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.
Alright, done.
Remark: sc_reload
is called on PDF file change and zathura_free
is called on program exit, but both of them calls document_close
anyway.
Side note 2, technically the non-null check is redundant because synctex_scanner_free
does the check anyway.
Fixes #685 .
The code is mostly copied from synctex's own code for new interactive feature.
A disadvantage I can think of is the scanner object consumes memory even when user is not looking up anything, but I think this is inevitable.