[Chandler-dev] Re: [commits] (heikki) [14591] Minor performance improvements:

D John Anderson john at osafoundation.org
Wed Jun 6 09:58:54 PDT 2007


Hi:

While I'm in favor of faster code, and I don't dispute that these  
changes make things faster, I'm curious if they result in any  
measurable or noticeable improvement.

See other comments inline below.

John

On Jun 6, 2007, at 9:45 AM, commits at osafoundation.org wrote:

> Revision
> 14591
> Author
> heikki
> Date
> 2007-06-06 09:45:39 -0700 (Wed, 06 Jun 2007)
> Log Message
>
> Minor performance improvements:
>
> * Removed some inappropriate usage of len() that showed in profiles
> * x,y,w,h=rect.Get() is faster than x,y,w,h=rect (and faster than  
> getting each value in a
> separate wxPython call)
> * Put some asserts that showed in profiles behind if __debug__: guards
> Modified Paths
>
> trunk/chandler/parcels/osaf/framework/attributeEditors/ 
> AttributeEditors.py
> trunk/chandler/parcels/osaf/framework/blocks/Block.py
> trunk/chandler/parcels/osaf/framework/blocks/Table.py
> trunk/chandler/parcels/osaf/framework/blocks/calendar/ 
> CalendarBlocks.py
> trunk/chandler/parcels/osaf/framework/blocks/calendar/ 
> CalendarCanvas.py
> trunk/chandler/parcels/osaf/views/main/Sections.py
> trunk/chandler/parcels/osaf/views/main/SideBar.py
> trunk/chandler/util/divisions.py
> Diff
>
> Modified: trunk/chandler/parcels/osaf/framework/attributeEditors/ 
> AttributeEditors.py (14590 => 14591)
>
> --- trunk/chandler/parcels/osaf/framework/attributeEditors/ 
> AttributeEditors.py	2007-06-06 16:14:16 UTC (rev 14590)
> +++ trunk/chandler/parcels/osaf/framework/attributeEditors/ 
> AttributeEditors.py	2007-06-06 16:45:39 UTC (rev 14591)
> @@ -1302,8 +1302,9 @@
>          if image is None:
>              logger.debug("Hey, missing image!")
>          if image is not None:
> -            x = rect.GetLeft() + (rect.GetWidth() - image.GetWidth 
> ()) / 2
> -            y = rect.GetTop() + (rect.GetHeight() - image.GetHeight 
> ()) / 2
> +            x, y, w, h = rect.Get()
> +            x += (w - image.GetWidth()) / 2
> +            y += (h - image.GetHeight()) / 2
>              dc.DrawBitmap(image, x, y, True)
>
>      def OnMouseChange(self, event):
> Modified: trunk/chandler/parcels/osaf/framework/blocks/Block.py  
> (14590 => 14591)
>
> --- trunk/chandler/parcels/osaf/framework/blocks/Block.py	 
> 2007-06-06 16:14:16 UTC (rev 14590)
> +++ trunk/chandler/parcels/osaf/framework/blocks/Block.py	 
> 2007-06-06 16:45:39 UTC (rev 14591)
> @@ -908,7 +908,7 @@
>                  title = getattr(sender, 'title', None)
>                  if title is not None:
>                      accel = getattr(sender, 'accel', u'')
> -                    if len(accel) > 0:
> +                    if accel:
>                          title += u'\t' + accel
>                          # this isn't a real wx argument, but is  
> used later
>                          # to re-attach the accelerator after the  
> client has
> @@ -1054,7 +1054,7 @@
>          #then the block is shown. If the attribute is missing then  
> the block is shown.
>          show = getattr (blockItem, "emptyContentsShow", None)
>          if show is not None:
> -            show = (len(blockItem.contents) != 0) ^ show
> +            show = bool(blockItem.contents) ^ show
>              if blockItem.isShown != show:
>                  blockItem.isShown = show
>                  self.Show (show)
> Modified: trunk/chandler/parcels/osaf/framework/blocks/Table.py  
> (14590 => 14591)
>
> --- trunk/chandler/parcels/osaf/framework/blocks/Table.py	 
> 2007-06-06 16:14:16 UTC (rev 14590)
> +++ trunk/chandler/parcels/osaf/framework/blocks/Table.py	 
> 2007-06-06 16:45:39 UTC (rev 14591)
> @@ -116,8 +116,9 @@
>              delegate = AttributeEditors.getSingleton (type)
>              attribute = self.defaultROAttribute
>              grid = self.GetView()
> -            assert (row < grid.GetTable().GetNumberRows() and
> -                    column < grid.GetTable().GetNumberCols())
> +            if __debug__:
> +                assert (row < grid.GetTable().GetNumberRows() and
> +                        column < grid.GetTable().GetNumberCols())
I think the if __debug__ can be removed, since asserts only happen  
when __debug__ is True.
>
>
>              if (not grid.blockItem.columns[column].readOnly and
>                  not grid.ReadOnly (row, column)[0] and
> @@ -542,7 +543,7 @@
>              # remember the first row in the old selection
>              topLeftSelection = self.GetSelectionBlockTopLeft()
>
> -            if len(topLeftSelection) > 0:
> +            if topLeftSelection:
>                  newRowSelection = topLeftSelection[0][0]
>              else:
>                  newRowSelection = -1
> @@ -796,7 +797,8 @@
>          """
>          DrawingUtilities.SetTextColorsAndFont (grid, attr, dc,  
> isInSelection)
>          value = grid.GetElementValue (row, column)
> -        assert len(value) != 2 or not value[0].isDeleted()
> +        if __debug__:
> +            assert len(value) != 2 or not value[0].isDeleted()
Same here
>
>          self.delegate.Draw(grid, dc, rect, value, isInSelection)
>
>  class GridCellAttributeEditor (wxGrid.PyGridCellEditor):
> Modified: trunk/chandler/parcels/osaf/framework/blocks/calendar/ 
> CalendarBlocks.py (14590 => 14591)
>
> --- trunk/chandler/parcels/osaf/framework/blocks/calendar/ 
> CalendarBlocks.py	2007-06-06 16:14:16 UTC (rev 14590)
> +++ trunk/chandler/parcels/osaf/framework/blocks/calendar/ 
> CalendarBlocks.py	2007-06-06 16:45:39 UTC (rev 14591)
> @@ -669,7 +669,7 @@
>          # Draw title if appropriate
>          times = []
>          timesCoords = []
> -        if self.useToday and len(self.visibleEvents) > 0:
> +        if self.useToday and self.visibleEvents:
>              times.append(_(u"Today's events"))
>              timesCoords.append((self.hMargin, y))
>              y += self.lineHeight
> Modified: trunk/chandler/parcels/osaf/framework/blocks/calendar/ 
> CalendarCanvas.py (14590 => 14591)
>
> --- trunk/chandler/parcels/osaf/framework/blocks/calendar/ 
> CalendarCanvas.py	2007-06-06 16:14:16 UTC (rev 14590)
> +++ trunk/chandler/parcels/osaf/framework/blocks/calendar/ 
> CalendarCanvas.py	2007-06-06 16:45:39 UTC (rev 14591)
> @@ -802,7 +802,7 @@
>          dc.DestroyClippingRegion()
>          dc.SetClippingRect(rect)
>
> -        (rectX,rectY,width,height) = rect
> +        (rectX,rectY,width,height) = rect.Get()
>          x = rectX
>          y = rectY
>
> @@ -1959,8 +1959,9 @@
>
>              changes = list(self.HandleRemoveAndYieldChanged 
> (currentRange))
>
> -            assert sorted(self.visibleEvents) == self.visibleEvents
> -            assert not self.HasPendingEventChanges()
> +            if __debug__:
> +                assert sorted(self.visibleEvents) ==  
> self.visibleEvents
> +                assert not self.HasPendingEventChanges()
and here
>
>
>              def syncWidget(w):
>                  w.wxHandleChanges(changes)
> Modified: trunk/chandler/parcels/osaf/views/main/Sections.py (14590  
> => 14591)
>
> --- trunk/chandler/parcels/osaf/views/main/Sections.py	2007-06-06  
> 16:14:16 UTC (rev 14590)
> +++ trunk/chandler/parcels/osaf/views/main/Sections.py	2007-06-06  
> 16:45:39 UTC (rev 14591)
> @@ -94,7 +94,7 @@
>              key=lambda x: getattr(x, self.attributeName))
>
>          # don't show section headers unless we have at least one  
> section
> -        if len(self.sectionIndexes) == 0:
> +        if not self.sectionIndexes:
>              return
>
>          # now build the row-based sections - each entry in this array
> @@ -137,9 +137,10 @@
>              self.sectionLabels.append(label)
>
>          # make sure we're sane
> -        assert len(self.sectionRows) == len(self.sectionIndexes)
> -        assert sum([visible+1 for (row, visible, total) in  
> self.sectionRows]) \
> -               == self.totalRows
> +        if __debug__:
> +            assert len(self.sectionRows) == len(self.sectionIndexes)
> +            assert sum([visible+1 for (row, visible, total) in  
> self.sectionRows]) \
> +                   == self.totalRows
and here
>
>
>      def findCurrentColumn(self):
>          # Find the column we're currently sorting by
> @@ -165,7 +166,7 @@
>          # temporary fix: when there are no sections (which is the
>          # condition under which the functional tests are running)  
> then
>          # use the original collection for row length
> -        if len(self.sectionRows) == 0:
> +        if not self.sectionRows:
>              return len(self.blockItem.contents)
>          else:
>              return self.totalRows
> @@ -214,7 +215,7 @@
>          won't optimize this.
>          """
>
> -        if len(self.sectionRows) == 0:
> +        if not self.sectionRows:
>              return row
>
>          sectionAdjust = len(self.sectionRows) - 1
> @@ -248,7 +249,7 @@
>          linear search through the sections. Generally there aren't a
>          lot of sections though so this should be reasonably fast.
>          """
> -        if len(self.sectionIndexes) == 0:
> +        if not self.sectionIndexes:
>              return itemIndex
>
>          sectionAdjust = len(self.sectionIndexes) - 1
> @@ -395,8 +396,10 @@
>          # they're contracted together... so erase a one-pixel-high  
> rectangle at
>          # the bottom of our rect, then make ours a little smaller,
>          dc.SetBrush(wx.WHITE_BRUSH)
> +        x, y, w, h = rect.Get()
>          rect.height -= 1
> -        dc.DrawRectangleRect((rect.x, rect.y + rect.height,  
> rect.width, 1))
> +        h -= 1
> +        dc.DrawRectangleRect((x, y + h, w, 1))
>
>          # Draw the background
>          brush = wx.Brush(sectionBackgroundColor, wx.SOLID)
> @@ -408,11 +411,11 @@
>          labelFont = Styles.getFont 
> (grid.blockItem.sectionLabelCharacterStyle)
>          dc.SetFont(labelFont)
>          (labelWidth, labelHeight, labelDescent, ignored) =  
> dc.GetFullTextExtent(label)
> -        labelTop = rect.y + ((rect.height - labelHeight) / 2)
> +        labelTop = y + ((h - labelHeight) / 2)
>
>          # Draw the expando triangle
>          (triBitmap, triWidth, triHeight) = self._getTriangle 
> (expanded)
> -        triTop = rect.y + ((rect.height - triHeight) / 2)
> +        triTop = y + ((h - triHeight) / 2)
>          dc.DrawBitmap(triBitmap, margin, triTop, True)
>
>          # Draw the text label, if it overlaps the rect to be updated
> @@ -440,8 +443,8 @@
>              dc.SetPen(wx.WHITE_PEN)
>              brush = wx.Brush(sectionSampleColor, wx.SOLID)
>              dc.SetBrush(brush)
> -            swatchX = rect.x + ((rect.width - swatchWidth) / 2)
> -            swatchY = rect.y + ((rect.height - swatchHeight) / 2)
> +            swatchX = x + ((w - swatchWidth) / 2)
> +            swatchY = y + ((h - swatchHeight) / 2)
>              dc.DrawRectangleRect((swatchX, swatchY, swatchWidth,  
> swatchHeight))
>
>      def OnMouseChange(self, event):
> Modified: trunk/chandler/parcels/osaf/views/main/SideBar.py (14590  
> => 14591)
>
> --- trunk/chandler/parcels/osaf/views/main/SideBar.py	2007-06-06  
> 16:14:16 UTC (rev 14590)
> +++ trunk/chandler/parcels/osaf/views/main/SideBar.py	2007-06-06  
> 16:45:39 UTC (rev 14591)
> @@ -1257,7 +1257,7 @@
>              # Finally, create a UI wrapper collection to manage
>              # things like selection and sorting
>              newKey = IndexedSelectionCollection 
> (itsView=self.itsView, source=key)
> -            if len (newKey) > 0:
> +            if len(newKey) > 0: # XXX if newKey: does not work;  
> other code depends on index being created here
>                  newKey.addSelectionRange (0)
>              UserCollection(newKey).dontDisplayAsCalendar =  
> UserCollection(key).dontDisplayAsCalendar
>              return newKey
> @@ -1298,7 +1298,7 @@
>                               theItem not in collectionList):
>                              collectionList.append (theItem)
>
> -            if len (collectionList) > 0:
> +            if collectionList:
>                  """
>                  tupleList is sorted so we always end up with one  
> collection
>                  for any order of collections in the source
> @@ -1330,7 +1330,7 @@
>                      key = None
>
>                  if (key is not None and
> -                    len ([c for c in collectionList if c.itsUUID  
> not in key.collectionList]) != 0):
> +                    [c for c in collectionList if c.itsUUID not in  
> key.collectionList]):
>                      # See bug #6793: If a subscribed collection  
> has been deleted, then resubscribed
>                      # our cached key will be stale because the  
> subscribed collection will have
>                      # been removed but not added when  
> resubscribed. In this case, the removed
> Modified: trunk/chandler/util/divisions.py (14590 => 14591)
>
> --- trunk/chandler/util/divisions.py	2007-06-06 16:14:16 UTC (rev  
> 14590)
> +++ trunk/chandler/util/divisions.py	2007-06-06 16:45:39 UTC (rev  
> 14591)
> @@ -101,11 +101,11 @@
>                  division_stack.append((i,j))
>
>      # this saves time
> -    if len(l) == 0:
> +    if not l:
>          return []
>
>      # the meat of it - the binary search
> -    while len(division_stack) != 0:
> +    while division_stack:
>
>          # pop the next range off the stack
>          index1, index2 = division_stack.pop()
>
> _______________________________________________
> Commits mailing list
> Commits at osafoundation.org
> http://lists.osafoundation.org/mailman/listinfo/commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.osafoundation.org/pipermail/chandler-dev/attachments/20070606/64ba8154/attachment.html


More information about the chandler-dev mailing list