On 01/29/2011 08:54 AM, Vincent Massol wrote:
>
> On Jan 29, 2011, at 3:59 AM, sdumitriu (SVN) wrote:
>
>> Author: sdumitriu
>> Date: 2011-01-29 03:59:38 +0100 (Sat, 29 Jan 2011)
>> New Revision: 34247
>>
>> Modified:
>>
>> platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/pdf/impl/PdfExportImpl.java
>> Log:
>> [refactoring] Better performance by avoiding repeated lookup of components
>>
>> Modified:
>> platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/pdf/impl/PdfExportImpl.java
>> ===================================================================
>> ---
>> platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/pdf/impl/PdfExportImpl.java
>> 2011-01-29 02:52:13 UTC (rev 34246)
>> +++
>> platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/pdf/impl/PdfExportImpl.java
>> 2011-01-29 02:59:38 UTC (rev 34247)
>> @@ -124,6 +124,12 @@
>> /** Logging helper object. */
>> private static final Log LOG = LogFactory.getLog(PdfExportImpl.class);
>>
>> + /** Velocity engine manager, used for interpreting velocity. */
>> + private static VelocityManager velocityManager =
>> Utils.getComponent(VelocityManager.class);
>> +
>> + /** The OpenOffice manager used for generating RTF. */
>> + private static OpenOfficeManager oooManager =
>> Utils.getComponent(OpenOfficeManager.class);
>
> This is not very since it will prevent the Extension Manager and the dynamic
> component mechanism to work (if a new component impl overrides the role/hint).
OK, I see, but this is not something done with the current component
injection, since an @Requirement doesn't change after the singleton
instance is initialized. I made this change preparing for a future
component, where the only change would be to replace the
Utils.getComponent call with @Requirement.
Since the PdfExportImpl class is a singleton as well, there's no
difference between static and instance at the moment. If you want, i can
remove the static declaration.
> We shouldn't use any static as much as possible. Note that a lookup doesn't
> cost anything since there's no synchronize and it's only a concurrent hashmap
> get, so I doubt it's going to improve perf by much (unless there's something
> I don't understand).
>
> Note that in the future an idea we've had with Thomas is that all
> @Requirement will actually be proxies to the component impl so that if the
> component is replaced you'll always get the current one.
Yes, this looks nice.
> Thanks
> -Vincent
>
>> +
>> /** DOM parser factory. */
>> private static DocumentBuilderFactory dbFactory =
>> DocumentBuilderFactory.newInstance();
>>
>> @@ -242,11 +248,9 @@
>> LOG.debug("Final XHTML for export: " + xhtml);
>> }
>>
>> - OpenOfficeManager OpenOfficeManager =
>> Utils.getComponent(OpenOfficeManager.class);
>> -
>> // If OpenOffice server is connected use it instead of FOP which
>> does not support RTF very well
>> // Only switch to openoffice server for RTF because FOP is
>> supposedly a lot more powerful for PDF
>> - if (type != PDF&& OpenOfficeManager.getState() ==
>> ManagerState.CONNECTED) {
>> + if (type != PDF&& oooManager.getState() == ManagerState.CONNECTED)
>> {
>> exportOffice(xhtml, out, type, context);
>> } else {
>> // XSL Transformation to XML-FO
>> @@ -285,10 +289,8 @@
>> // id attribute on body element makes openoffice converter to fail
>> html = html.replaceFirst("(<body[^>]+)id=\"body\"([^>]*>)", "$1$2");
>>
>> - OpenOfficeManager OpenOfficeManager =
>> Utils.getComponent(OpenOfficeManager.class);
>> + OpenOfficeConverter documentConverter = oooManager.getConverter();
>>
>> - OpenOfficeConverter documentConverter =
>> OpenOfficeManager.getConverter();
>> -
>> String inputFileName = "export_input.html";
>> String outputFileName = "export_output" + (type ==
>> PdfExportImpl.RTF ? ".rtf" : ".pdf");
>>
>> @@ -605,7 +607,6 @@
>> Utils.getComponent(EntityReferenceSerializer.class);
>> try {
>> StringWriter writer = new StringWriter();
>> - VelocityManager velocityManager =
>> Utils.getComponent(VelocityManager.class);
>> VelocityEngine engine =
>> velocityManager.getVelocityEngine();
>> try {
>> VelocityContext vcontext =
>> velocityManager.getVelocityContext();
>
--
Sergiu Dumitriu
http://purl.org/net/sergiu/
_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs
