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

[main-] remove unneeded reload in batch mode #2361

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

midichef
Copy link
Contributor

@midichef midichef commented Mar 25, 2024

Batch mode (when not replaying commands with -p) is substantially slower to load a file than interactive mode.

seq 2000111 > 2m.tsv
# interactive mode:  quit.vdj is a timing helper script that opens 2m.tsv and immediately quits
=time vd -p quit.vdj
6.41user
# batch mode
=time vd -b 2m.tsv
9.41user

quit.vdj.txt

That's because batch mode runs reload() twice in a row in these two lines:

visidata/visidata/main.py

Lines 336 to 337 in 581cdf1

vd.push(sources[0])
sources[0].reload()

vd.push() calls BaseSheet.ensureLoaded() which calls reload():

vs.ensureLoaded()

return self.reload() # likely launches new thread

This PR removes the unneeded explicit call to reload(), running 2x faster for the special case of no commands.

=time vd -b 2m.tsv
4.73user

Copy link
Owner

@saulpw saulpw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line was added in 330117f. I agree that it shouldn't be necessary, and a 2x slowdown is bad of course, but let's make sure that we can still read from stdin in batch mode as per #354.

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

Successfully merging this pull request may close these issues.

None yet

2 participants