On Jan 29, 2011, at 2:33 PM, Sergiu Dumitriu wrote:
> 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.
No that's ok. I just wanted to make sure we all understand the limitation and
the path to the future.
Thanks
-Vincent
>> 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();
_______________________________________________
devs mailing list
[email protected]
http://lists.xwiki.org/mailman/listinfo/devs
