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).

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.

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

Reply via email to