[nvda-addons] Re: commit/StationPlaylist: josephsl: 7.0 optimization: Make sure the foreground object has more than one children.

  • From: "Joseph Lee" <joseph.lee22590@xxxxxxxxx>
  • To: <nvda-addons@xxxxxxxxxxxxx>, <nvda-addons-commits@xxxxxxxxxxxxx>
  • Date: Mon, 22 Feb 2016 23:03:23 -0800

Hi folks,

The following may seem a bit technical, but I'll try my best to explain what
I've been doing for the benefit of those who've just joined us. But first, a
few things to note:
* The NVDA add-ons list is the central forum where we discuss NVDA community
add-ons. We do accept general user questions, but most posts (including the
one that follows) deals with add-ons. These include promoting an add-on,
requesting add-on reviews and inclusion into the add-ons website, important
communication between add-on authors and users, coordination of add-on
releases (new and maintenance versions) and so on.
* Just like the NVDA users list, we do not accept cracked software and
pirated product keys and/or add-ons.
* Current add-on reviewers are Joseph Lee (author of SPL, GoldWave, WinTen
App Essentials and other add-ons, the admin of this list, quarterly release
manager), Noelia Martinez (author of Place Markers, Clip Contents Designer
and others) and volunteer reviewers. We focus on different areas: I look at
user experience, translatable strings, user interface and GUI side of
things, Noelia looks at code readability and translations workflow, and
others look at different aspects of the add-on.
* Because this is a forum about technical artifacts (NVDA community
add-ons), some posts are technical in nature (in order to write add-ons, you
need to have experience with Python 2.7; those using 3.x are welcome, but
due to dependencies, NVDA Core uses 2.7).

Now to the topic at hand: for those following the commits list may have seen
commits to StationPlaylist Studio add-on repository marked as
"optimizations". Here's the story behind it:
When I sat down with SPL users a few months ago, I asked if the upcoming
add-on version (7.0) should be designated a long-term support (LTS)_
release. I told users that if 7.0 becomes an LTS release, this will be the
last version to support oldest stable Studio version. Many users agreed with
this idea, with one user saying that the latest Studio versions are quite
unstable at the moment, to which I promised that 7.0 will be supported for a
very long time (until end of 2017, according to the timetable I have).
As part of this plan, I decided to devote February as optimization month.
Over the years, the SPL add-on code have accumulated hacks, workarounds and
bug fixes that were great at the time of their development but are quite
outdated. Also, due to code accumulation, parts of the add-on have become
slower.
The optimization activities (which are complete and are awaiting integration
back into master branch) were as follows:
1. Marked areas of code that had potential to be optimized. This included
loops, overlay class chooser, redundant functions and so on (this wasn't
limited to the app module itself; areas examined include configuration
dialog code).
2. I inserted benchmarking code (time.time) to measure performance of the
add-on. This revealed that the add-on was doing unacceptable things such as
generating the same list over and over again through each iteration of a
loop (this was done using list comprehension, and effectively this meant a
polynomial time algorithm was in use).
3. Combined functions. Functions are great when you need to split workloads,
but sometimes you find that this function is used in one specific place (or
nowhere at all).
4. Took time to rewrite certain portions of the add-on to promote code
reuse.
5. Tested optimization ideas with users.

Specifically:
* reportFocus in SPLTrackItem class (app module): ReportFocus is the
function responsible for reporting the currently focused control. In fact,
this function is called when gainFocus event takes place. In case of the SPL
app module, reportFocus is responsible for announcing track columns in
specific order if different from the screen presentation order.
Previously, the algorithm was as follows:
1. Steps through list of columns.
2. Through each iteration, a private function is used to locate the index of
this column (this index tells NVDA which column to announce).
3. The index function generates list of columns that are shown on screen
(done via list comprehension). The index is then returned to the for loop.
It turns out this was actually an "N squared" algorithm and was incorrect
and redundant from the beginning: polynomial because another list was
generated through each iteration of the loop, and list iteration had to be
done in order to locate the correct column index (list.index); it was wrong
and redundant because no matter how the columns are arranged in Studio 5.10
and later, internal representation and column position does not change
(repeated calls to column content getter which used kernel32.dll's
VirtualAllocEx/WriteProcessMemory/ReadProcessMemory/VirtualFreeEx (same
trick as SysListView32 support) confirmed this hypothesis. The trick in this
optimization is noting that each Studio track item class knows how to obtain
column header indices, and all the index function will now do is ask the
track class to use its own method of obtaining column position for a column
header. This improved reportFocus speed with custom column presentation
order set by up to four times, with the added benefit that Track Dial
(NVDA's enhanced arrow keys feature) can now work with columns no matter how
they are arranged on screen.

* Code reuse: in Studio add-on 5.6, I've added a routine to announce name of
the playing track from other programs. In current implementation (applies to
add-on 6.x), nameChange event handler walks the object hierarchy to make
sure the just changed name comes from the text field which records track
names. Not only this was ugly to look at, but it meant I had to tune this
for each Studio version.
Not anymore thanks to provisions made many years ago: there is a function in
the app module class that is designed to obtain specific objects given its
position in relation to the Studio window, and I taught this function how to
locate track titles. The optimization that was committed today uses this
trick: the name change event handler now compares the status object to see
if it is indeed the track title field (improving readability and promoting
code reuse along the way).

Few advice on code optimizations:
1. Never optimize until your add-on works as advertised. A famous computer
scientist once said, "premature optimization is root of all evil". You don't
want to focus a lot on optimization in the beginning - make sure a feature
works, and then think about optimizing it later when you have time. I'd
advise you to not optimize your code in add-on 1.0 - maybe in 3.0 or later.
2. Have a backup of your code before optimizing. For those using Git, this
is where branching shines. For those not using source code management
package, the best way to ensure code backup is using multiple copies of
NVDA. Having one or more backups ensures easy way out if something goes
wrong during optimization.
3. Test and mark places that truly needs optimizing. Go through your add-on
code until you find a place that you think optimization may help. When
marking code for optimization, justify why optimization is necessary
(preferably after many tests), and don't try to think about multiple
optimization techniques in one go (you need to go through your code multiple
times if you'd like to apply multiple optimizations in certain parts of the
module unless you have solid ideas on what needs to be done).
4. Do one optimization at a time. Even if this means multiple commits, it is
best to do one thing at a time so you can backtrack easily (for Git users,
git revert helps).
5. Document your optimization. Although some will say excessive
documentation is a noise, I am a firm believer in power of good-quality
source code comments and product documentation (I'll reserve discussion of
add-on code comments and readme for a separate thread later). Write before
and after pictures, what you've learned through tests and optimizations and
so on (preferably as part of the commit message; for this reason, one of my
pet peeves is really brief commit messages like "removed a script" or "code
change"). After all, computer programming is a form of writing (for members
who are students, I'm sure you'll say, "writing an essay is hard"; think
about writing technical documentation, which is harder than writing many
paragraphs on general subjects).
6. Test, test, test. There is an anecdote where Louis Armstrong said to
someone asking for direction to a concert hall, "practice, practice,
practice. Please remember that your optimization(s) will be "felt" by users,
so you need to test your ideas to make sure it works as advertised. In other
words, you yourself must become an accomplished user of your own add-on
before telling users that your optimization is a great success (this
requires devoting time to test your before and after code). Also, it is a
good practice to invite your users along to test your ideas and hear what
they have to say (Roger Stewart knows about this, as he is one of the early
adapters of SPL add-on features), and because of this, I'll hold a session
during NVDACon 2016 where users of my add-ons can talk to me and give me
feedback (and I'd like to advise other add-on authors to do the same so you
can talk to users of your add-ons).
7. If possible, don't optimize unless you REALLY have to. To put it the
other way, invest time in good code design when writing add-ons or new
add-on features. As written in the add-on development guide (which I need to
update to include audio ducking support), be sure to plan ahead, which
implies good design from the beginning, which may allow you to optimize your
code easily (for this reason, I usually don't start writing Python
statements until I have an idea on what to write and think about future
versions).

I think we may need to add a note on optimization in our add-on development
guide...
Good luck. As always, if you have questions, feel free to ask on the list.
Cheers,
Joseph





-----Original Message-----
From: nvda-addons-commits-bounce@xxxxxxxxxxxxx
[mailto:nvda-addons-commits-bounce@xxxxxxxxxxxxx] On Behalf Of
commits-noreply@xxxxxxxxxxxxx
Sent: Monday, February 22, 2016 9:26 PM
To: nvda-addons-commits@xxxxxxxxxxxxx
Subject: commit/StationPlaylist: josephsl: 7.0 optimization: Make sure the
foreground object has more than one children.

1 new commit in StationPlaylist:

https://bitbucket.org/nvdaaddonteam/stationplaylist/commits/906e75595b3a/
Changeset:   906e75595b3a
Branch:      7.0/blackOptimizations
User:        josephsl
Date:        2016-02-23 05:24:52+00:00
Summary:     7.0 optimization: Make sure the foreground object has more than
one children.

Even if we are working with a known foreground object, sometimes it only has
a single child. If so, it causes index error to be thrown (status function).
This was observed mostly in Studio 5.1x.
This completes current optimization steps (destined for 7.0).

Affected #:  1 file

diff --git a/addon/appModules/splstudio/__init__.py
b/addon/appModules/splstudio/__init__.py
index 52d2c69..8e5a08a 100755
--- a/addon/appModules/splstudio/__init__.py
+++ b/addon/appModules/splstudio/__init__.py
@@ -1416,7 +1416,7 @@ class AppModule(appModuleHandler.AppModule):
                        if not self.productVersion >= "5.10": statusObj =
self.statusObjs[infoIndex][0]
                        else: statusObj = self.statusObjs[infoIndex][1]
                        # 7.0: sometimes (especially when first loaded),
OBJID_CLIENT fails, so resort to retrieving focused object instead.
-                       if fg is not None:
+                       if fg is not None and fg.childCount > 1:
                                self._cachedStatusObjs[infoIndex] =
fg.children[statusObj]
                        else: return api.getFocusObject()
                return self._cachedStatusObjs[infoIndex]

Repository URL: https://bitbucket.org/nvdaaddonteam/stationplaylist/

--

This is a commit notification from bitbucket.org. You are receiving this
because you have the service enabled, addressing the recipient of this
email.

----------------------------------------------------------------
NVDA add-ons: A list to discuss add-on code enhancements and for reporting 
bugs. 

Community addons are available from: http://addons.nvda-project.org
To send a message to the list: nvda-addons@xxxxxxxxxxxxx
To change your list settings/unsubscribe: 
//www.freelists.org/list/nvda-addons
To contact list moderators: nvda-addons-moderators@xxxxxxxxxxxxx

Other related posts:

  • » [nvda-addons] Re: commit/StationPlaylist: josephsl: 7.0 optimization: Make sure the foreground object has more than one children. - Joseph Lee