Issue 51304 - svx/source/form/fmshell.cxx: performance
Summary: svx/source/form/fmshell.cxx: performance
Status: CLOSED FIXED
Alias: None
Product: Draw
Classification: Application
Component: code (show other issues)
Version: 680m110
Hardware: Other All
: P3 Trivial (vote)
Target Milestone: OOo 2.0
Assignee: Frank Schönheit
QA Contact: issues@graphics
URL:
Keywords: oooqa
Depends on:
Blocks: 20269
  Show dependency tree
 
Reported: 2005-06-28 12:26 UTC by mmeeks
Modified: 2008-05-16 03:31 UTC (History)
2 users (show)

See Also:
Issue Type: PATCH
Latest Confirmation in: ---
Developer Difficulty: ---

Attachments
go faster stripes ... (5.24 KB, patch)
2005-06-28 16:05 UTC, mmeeks 		no flags Details | Diff
updated patch (10.26 KB, patch)
2005-07-01 12:33 UTC, mmeeks 		no flags Details | Diff
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this issue.
Description mmeeks 2005-06-28 12:26:53 UTC 
FmFormShell::HasUIFeature( sal_uInt32 nFeature )
...
        else if ((nFeature & FM_UI_FEATURE_SHOW_DATANAVIGATOR) == 
                 FM_UI_FEATURE_SHOW_DATANAVIGATOR)
        {
	    access ("foo baa: started", 0);
            bResult = ( GetImpl()->getDocumentType() == eEnhancedForm );
	    access ("foo baa: finished", 0);
        }
...
I get ~100ms of work (Linux) done [ strace -ttt output ] between those two
accesses. Worse - this links in the large & not-visibility-marked-up 'dbtools'
which (as an extra library to search) will further degrade subsequent linkage.

The worst thing is - all that library loading goes to waste - since we only care
about the boolean == eEnhancedForm state - which a simple query for
XFormsSupplier will tell us ;-)

I'll knock up a patch in an hour or two to add a quicker 'isEnhancedFoo' helper
 method there ...
Comment 1 mmeeks 2005-06-28 16:05:43 UTC 
Created attachment 27543 [details]
go faster stripes ...
Comment 2 mmeeks 2005-06-28 16:06:10 UTC 
rather a trivial fix.
Comment 3 christian.guenther 2005-06-29 15:07:01 UTC 
One for you.
Please have a look.
Comment 4 Frank Schönheit 2005-06-29 15:16:34 UTC 
- Please rename isDocumentEnhanced to isEnhancedForm, for reasons of
  consistency (and, well, better fitting semantics)
- In FmXFormShell::isEnhancedForm, please don't duplicate the code from
  FmXFormShell::getDocumentType, but move it to a common method
  (getContextDocument or something like this)
  Duplicated code has a tendency to get out of sync, by fixing bugs in one instance
  only.
- There are several more places in fmshell.cxx which simply compare
  getDocumentType with eEnhancedForm. They potentially bear the same problem,
  so please adjust them to use the new method, too.
  Same for one line in fmview.cxx and navigatortree.cxx, respectively.

Other than that, I'd be fine with the patch.
Comment 5 mmeeks 2005-07-01 09:46:32 UTC 
I'll re-work the patch a little today for you if I can :-)
Comment 6 mmeeks 2005-07-01 12:33:26 UTC 
Created attachment 27618 [details]
updated patch
Comment 7 Frank Schönheit 2005-07-01 12:50:54 UTC 
thanks :), fine with me now

I currently only have one 2.0-CWS which is already in QA, so I cannot easily
check in there. Do you have a CWS at hand?
Comment 8 mmeeks 2005-07-04 11:39:39 UTC 
sure - committing to mmeeks09; I'll mark you as qa until we get a better idea.
Comment 9 mmeeks 2005-07-04 12:09:44 UTC 
fixed & marked 'ready for QA'
Comment 10 Frank Schönheit 2005-07-05 08:07:53 UTC 
don't have access to an installation set of the CWS, but applied the patch to
one of my installations, and everything looks fine.
Comment 11 ace_dent 2008-05-16 03:31:59 UTC 
This Issue is 'Verified' and not updated in 1yr+, so Closing.
A Closed Issue is a Happy Issue (TM).

Regards,
Andrew
 
Cleaning-up and Closing old Issues as part of:
~ The Grand Bug Squash, pre v3 ~