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

Speed up jrnl by 10%, improve slow imports #959

Merged
merged 5 commits into from
May 27, 2020
Merged

Conversation

wotgl
Copy link
Contributor

@wotgl wotgl commented May 20, 2020

Hi!

I noticed, that jrnl starts with a slight delay. I researched code and found some slow and rare global imports.
I made rare imports local and added an additional function, which checks version of jrnl.

These improvements have boosted speed by 10% and have boosted python imports by 23%. I used
perf stat and python3 -X importtime for analyzing.

Perf stat on the develop branch:

[user@pc jrnl]$ echo "" > /home/user/.local/share/jrnl/journal.txt
[user@pc jrnl]$ perf stat -r 100 -d ./run test

 Performance counter stats for './run test' (100 runs):

            254,69 msec task-clock:u              #    0,993 CPUs utilized            ( +-  0,67% )
                 0      context-switches:u        #    0,000 K/sec                  
                 0      cpu-migrations:u          #    0,000 K/sec                  
             3 363      page-faults:u             #    0,013 M/sec                    ( +-  0,05% )
       377 839 305      cycles:u                  #    1,484 GHz                      ( +-  0,34% )  (60,78%)
       485 808 775      instructions:u            #    1,29  insn per cycle           ( +-  0,20% )  (74,92%)
       101 730 359      branches:u                #  399,435 M/sec                    ( +-  0,13% )  (75,89%)
         3 194 572      branch-misses:u           #    3,14% of all branches          ( +-  0,20% )  (76,18%)
       136 039 720      L1-dcache-loads:u         #  534,148 M/sec                    ( +-  0,29% )  (76,24%)
         7 853 820      L1-dcache-load-misses:u   #    5,77% of all L1-dcache hits    ( +-  0,40% )  (76,28%)
         2 254 441      LLC-loads:u               #    8,852 M/sec                    ( +-  0,62% )  (47,87%)
           399 044      LLC-load-misses:u         #   17,70% of all LL-cache hits     ( +-  2,48% )  (47,54%)

           0,25655 +- 0,00190 seconds time elapsed  ( +-  0,74% )

Perf stat with my improvements:

[user@pc jrnl]$ echo "" > /home/user/.local/share/jrnl/journal.txt
[user@pc jrnl]$ perf stat -r 100 -d ./run test

 Performance counter stats for './run test' (100 runs):

            228,56 msec task-clock:u              #    0,993 CPUs utilized            ( +-  0,83% )
                 0      context-switches:u        #    0,000 K/sec                  
                 0      cpu-migrations:u          #    0,000 K/sec                  
             2 981      page-faults:u             #    0,013 M/sec                    ( +-  0,11% )
       337 848 080      cycles:u                  #    1,478 GHz                      ( +-  0,40% )  (60,34%)
       438 910 527      instructions:u            #    1,30  insn per cycle           ( +-  0,22% )  (73,59%)
        90 813 645      branches:u                #  397,327 M/sec                    ( +-  0,18% )  (73,62%)
         2 918 506      branch-misses:u           #    3,21% of all branches          ( +-  0,18% )  (73,74%)
       133 304 455      L1-dcache-loads:u         #  583,232 M/sec                    ( +-  0,37% )  (74,73%)
         6 939 200      L1-dcache-load-misses:u   #    5,21% of all L1-dcache hits    ( +-  0,34% )  (77,41%)
         2 235 461      LLC-loads:u               #    9,781 M/sec                    ( +-  0,36% )  (51,65%)
           510 868      LLC-load-misses:u         #   22,85% of all LL-cache hits     ( +-  0,91% )  (48,84%)

           0,23022 +- 0,00195 seconds time elapsed  ( +-  0,85% )

And some screenshots for importtime measures
Develop: https://imgur.com/lk4AslE
My improvements: https://imgur.com/0snmcEo

Checklist

  • The code change is tested and works locally.
  • Tests pass. Your PR cannot be merged unless tests pass. --
    poetry run behave
  • The code passes linting via
    black (consistent code styling). --
    poetry run black --check . --verbose --diff
  • The code passes linting via pyflakes
    (logically errors and unused imports). -- poetry run pyflakes jrnl features
  • There is no commented out code in this PR.
  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open
    Pull Requests for the same update/change?
  • Have you added an explanation of what your changes do and why you'd like
    us to include them?
  • Have you written new tests for your core changes, as applicable?

Copy link
Member

@wren wren left a comment

Choose a reason for hiding this comment

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

This looks great. Just a few small changes, and we should be good to go. Thank you!

)
print("Exiting.", file=sys.stderr)
sys.exit(1)
if util.is_old_version(config_path):
Copy link
Member

Choose a reason for hiding this comment

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

I agree that this is good to break out into a separate function, but without also updating upgrade_jrnl_if_necessary we now perform this check twice.

jrnl/util.py Outdated
def is_config_json(config_path):
with open(config_path, "r", encoding="utf-8") as f:
config_file = f.read()
if not config_file.strip().startswith("{"):
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick:

if not condition:
  return False
return True

can be more succinctly written as

return condition

@wotgl
Copy link
Contributor Author

wotgl commented May 23, 2020

Hi @wren ,

Thanks for your comments! I improved code in the last commit, check it, please :)

Copy link
Member

@wren wren left a comment

Choose a reason for hiding this comment

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

Thank you! 🏅

@wren wren added the enhancement New feature or request label May 27, 2020
@wren wren changed the title Improve slow imports, speedup by 10% Speed up jrnl by 10%, improve slow imports May 27, 2020
@wren wren merged commit a077660 into jrnl-org:develop May 27, 2020
wren pushed a commit that referenced this pull request Jul 25, 2020
* Improve slow imports
* Fix codestyle
* fix twice version validation
* Fix a syntax mistake
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants