AAClause / BrailleExtender

NVDA add-on that improves braille support
https://andreabc.net/projects/NVDA_addons/BrailleExtender/
GNU General Public License v2.0
16 stars 17 forks source link

Code refactoring: edit various components to make them conform with Python idioms and to optimize for speed #70

Open josephsl opened 4 years ago

josephsl commented 4 years ago

Hi,

The following will require extensive testing as it concerns optimizations:

Although Braille Extender works beautifully, there are many places where it can benefit from code rewrite to conform with Python idiom, syntax suggestions, and optimize for speed. For example, when one obtains an overview of the currently active input and output braille tables, a function named utils.combinationDesign will be run. Part of its job is to convert braille dots to a representation suitable for output. At the moment a while loop is used, but it was found that z for loop or a subtle optmization would benefit readability and speed.

Note on speed: in order to truly test the effectiveness of speed optimizations, one must:

  1. Benchmark the code path in question multiple times (preferably with time.time() and friends).
  2. In order to truly optimize for speed, bytecode disassembly is a must.

To illustrate the second point, in combination design function found in utils module, each iteration of the while loop will do the following:

  1. Before while loop is invoked, dot (index) is set to 1.
  2. Current dot value (represented by the variable "i") will be converted into a string, not once but twice (first to obtain the dot, second time to check membership).
  3. The eventual result string (represented by the variable "out") will be concatenated either with the dot value or unicode for dots 3-6.

The function takes 31 Python virtual machine instructions. Converting this into a for loop results in 31 instructions but with the following change: instead of assigning the variable i to 1 in the beginning, it is incremented as part of the for loop with the range set to (1, 9). This makes the code cleaner.

But there is an optimization opportunity: what if instead of treating the dots as integers, treat them as a character? This results in:

  1. Instead of a range-based for loop or a traditional integer-based while loop, the variable i will iterate over a string (string is a container as far as Python's iteration protocol is concerned).
  2. No more integer to string conversion as data can be compared directly.

This results in the function using only 25 Python virtual machine instructions. Because combination design function will be called whenever a dot combination exists in the input and output table, this optimization results in savings of hundreds of bytecode instructions. In order to do this kind of optimization, bytecode disassembly is a must.

There are many other places where this kind of edits would benefit the add-on, not only for code readability but also for future maintenance. As for when the accompanying pull request will show up, it won't be done until #69 is done.

P.S. while at it, I propose adding source code comments (finally).

Thanks.

AAClause commented 4 years ago

Hi Joseph, Thanks a lot for all your issues/suggestions. Know that I'm very glad to receive your comments. Indeed many things can be optimized. Some portions of the code come from my beginnings in Python and are unfortunately not optimized/very poorly written. I'm totally agree to change that. :) Currently, I am actively working on #56 and #64. But I am OK to work on this after those (probably from August). Feel free to make pull requests/comments!

Thanks again and good luck with my code :)

josephsl commented 4 years ago

Hi, I can wait for those two issues to be resolved before merging all this work. In the meantime, let me know about parts that you think may need some help in optimizing or rewrites, and I’ll work on converting scripts to script decorators (separate issue). Thanks.

From: André-Abush Clause notifications@github.com Sent: Thursday, July 16, 2020 1:42 AM To: Andre9642/BrailleExtender BrailleExtender@noreply.github.com Cc: Joseph Lee joseph.lee22590@gmail.com; Author author@noreply.github.com Subject: Re: [Andre9642/BrailleExtender] Code refactoring: edit various components to make them conform with Python idioms and to optimize for speed (#70)

Hi Joseph, Thanks a lot for all your issues/suggestions. Know that I'm very glad to receive your comments. Indeed many things can be optimized. Some portions of the code come from my beginnings in Python and are unfortunately not optimized/very poorly written. I'm totally agree to change that. :) Currently, I am actively working on #56 https://github.com/Andre9642/BrailleExtender/pull/56 and #64 https://github.com/Andre9642/BrailleExtender/pull/64 . But I am OK to work on this after those (probably from August). Feel free to make pull requests/comments!

Thanks again and good luck with my code :)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Andre9642/BrailleExtender/issues/70#issuecomment-659254329 , or unsubscribe https://github.com/notifications/unsubscribe-auth/AB4AXEG5LCM2ANT4R6DVKXDR324MVANCNFSM4O3XWTGA .