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

PICARD-1204: Read and dump values to QSettings only when necessary #854

Merged
merged 4 commits into from Mar 1, 2018

Conversation

@samj1912
Copy link
Member

@samj1912 samj1912 commented Mar 1, 2018

Reading and dumping too often from QSettings causes picard to lock up.
This makes sure we only read on start and dump on change.

Summary

  • This is a…
    • Bug fix
    • Feature addition
    • Refactoring
    • Minor / simple change (like a typo)
    • Other
  • Describe this change in 1-2 sentences:

Problem

Solution

Action

@samj1912 samj1912 changed the title PICARD-1204: Read and dump values to QtConfig only when necessary PICARD-1204: Read and dump values to QSettings only when necessary Mar 1, 2018
@samj1912 samj1912 force-pushed the picard_1204 branch 3 times, most recently from 7da4fdc to 20fdc07 Mar 1, 2018
samj1912 added 2 commits Mar 1, 2018
Reading and dumping too often from QSettings causes picard to lock up.
This makes sure we only read on start and dump on change.
picard/config.py Outdated
self.load_keys()

def __qt_keys(self):
return filter(lambda key: key.startswith('%s/' % self.__name),
Copy link
Collaborator

@zas zas Mar 1, 2018

Choose a reason for hiding this comment

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

You may want to cache '%s/' % self.__name:

def __qt_keys(self):
   prefix = '%s/' % self.__name
   def prefixed(key):
       return key.startswith(prefix)
    return filter(prefixed, self.__qt_config.allKeys())

Loading

picard/tagger.py Outdated
@@ -230,7 +231,6 @@ def __init__(self, picard_args, unparsed_args, localedir, autoupdate):
self.unclustered_files = UnclusteredFiles()
self.nats = None
self.window = MainWindow()
self.exit_cleanup = []
Copy link
Collaborator

@zas zas Mar 1, 2018

Choose a reason for hiding this comment

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

This change isn't needed anymore.

Loading

@samj1912 samj1912 force-pushed the picard_1204 branch 2 times, most recently from bdd62e8 to ca9eaad Mar 1, 2018
picard/config.py Outdated
self.load_keys()

def __qt_keys(self):
prefix = '%s/' % self.__name
Copy link
Collaborator

@zas zas Mar 1, 2018

Choose a reason for hiding this comment

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

Instead of using % format, since key building here are very simple, using concatenation will waste less cpu cycles.
Since config options can be read in loops, optimization makes sense, and doesn't cost much in readibility.

prefix = '%s/' % self.__name
key = "%s/%s" % (self.__name, name)

vs

prefix = self.__name + '/'
key = self.__name + '/' + name
from timeit import timeit

runs = 10000000


def print_results(time, start_string):
    print('%s\nTotal: %.4f\nAvg: %.4fns\n' % (start_string, time,
                                              (time/runs)*1000000000))

t1 = timeit('"%s/%s" % (a, b)',
            setup='a="xxx"; b="yyy"',
            number=runs)
t2 = timeit("a + '/' + b",
            setup='a="xxx"; b="yyy"',
            number=runs)
t3 = timeit('a + "/" + b',
            setup='a="xxx"; b="yyy"',
            number=runs)
t4 = timeit('"{}/{}".format(a, b)',
            setup='a="xxx"; b="yyy"',
            number=runs)

print_results(t1, '% replacement')
print_results(t2, 'concatenation single quotes')
print_results(t3, 'concatenation double quotes')
print_results(t4, '.format method')
% replacement
Total: 3.1121
Avg: 311.2064ns

concatenation single quotes
Total: 1.6318
Avg: 163.1769ns

concatenation double quotes
Total: 1.7592
Avg: 175.9204ns

.format method
Total: 4.3462
Avg: 434.6209ns

Loading


def raw_value(self, key):
def raw_value(self, name):
Copy link
Member Author

@samj1912 samj1912 Mar 1, 2018

Choose a reason for hiding this comment

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

Did this so that there is a consistency between name and key

Loading

Copy link
Collaborator

@zas zas Mar 1, 2018

Choose a reason for hiding this comment

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

Ok ;)

Loading

zas
zas approved these changes Mar 1, 2018

def raw_value(self, key):
def raw_value(self, name):
Copy link
Collaborator

@zas zas Mar 1, 2018

Choose a reason for hiding this comment

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

Ok ;)

Loading

@samj1912 samj1912 merged commit 61d5550 into metabrainz:master Mar 1, 2018
3 checks passed
Loading
@samj1912 samj1912 deleted the picard_1204 branch Mar 1, 2018
zas added a commit to zas/picard that referenced this issue Mar 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants