Skip to content

Commit 9b994af

Browse files
committed
Only do "task --version" once in TaskWarriorShellout() class
A TaskWarriorShellout instance calls .get_version() already thrice: once when determining if we .can_use() TaskWarriorShellout rather than TaskWarriorDirect, and then twice in the instance constructor itself. Therefore creation of the instance already calls it thrice. Each of these ends up executing "task --version" with execve(2) via subprocess.Popen(). Use of the methods .sync() or .filter_tasks() would execute this once again, while using .task_add() would do it twice. We therefore end up executing "task --version" four times for a single filter_tasks() and five times for a task_add(). Scripts which are doing multiple filters are constantly adding the overhead of these fork/execve just to obtain the version. My own personal taskgrep wrapper is making several calls with different filters, and strace is showing 18 different "task --version" calls, which on my modern-ish laptop adds about a tenth of a second to my taskgreps: $ TIMEFORMAT='time: %3Rs'; time \ for ((i = 0; i < 18; i++)) do task --version &>/dev/null; done time: 0.089s It's not huge, but these unncessary repeated "task --version" calls really can just be done once and the value cached as a class attribute Note that we cannot do this in the instance constructor, because can_use() is a class method called at module import time, before any instance will have been created, so we need get_version() to continue to be a class method. Thanks also to @ryaneeverett for suggesting to use a simple cached class attribute in get_version() rather than the first implementation which moved these statements into the class definition.
1 parent 3bd8ee9 commit 9b994af

File tree

1 file changed

+5
-2
lines changed

1 file changed

+5
-2
lines changed

taskw/warrior.py

+5-2
Original file line numberDiff line numberDiff line change
@@ -561,16 +561,19 @@ def can_use(cls):
561561

562562
@classmethod
563563
def get_version(cls):
564+
if hasattr(cls, 'taskwarrior_version'):
565+
return cls.taskwarrior_version
564566
try:
565-
taskwarrior_version = subprocess.Popen(
567+
taskwver = subprocess.Popen(
566568
['task', '--version'],
567569
stdout=subprocess.PIPE
568570
).communicate()[0]
569571
except FileNotFoundError:
570572
raise FileNotFoundError(
571573
"Unable to find the 'task' command-line tool."
572574
)
573-
return LooseVersion(taskwarrior_version.decode())
575+
cls.taskwarrior_version = LooseVersion(taskwver.decode())
576+
return cls.taskwarrior_version
574577

575578
def sync(self, init=False):
576579
if self.get_version() < LooseVersion('2.3'):

0 commit comments

Comments
 (0)