Re: [xwiki-devs] [xwiki-notifications] r34247 - platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/pdf/impl
29 Jan
2011
29 Jan
'11
8:54 a.m.
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();
29 Jan
29 Jan
2:33 p.m.
New subject: [xwiki-devs] [xwiki-notifications] r34247 - platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/pdf/impl
On 01/29/2011 08:54 AM, Vincent Massol wrote:
On Jan 29, 2011, at 3:59 AM, sdumitriu (SVN) wrote:
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.
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.
Yes, this looks nice.
Thanks
-Vincent
--
Sergiu Dumitriu
http://purl.org/net/sergiu/
+
/** 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();
31 Jan
31 Jan
11:31 a.m.
New subject: [xwiki-devs] [xwiki-notifications] r34247 - platform/core/trunk/xwiki-core/src/main/java/com/xpn/xwiki/pdf/impl
On Jan 29, 2011, at 2:33 PM, Sergiu Dumitriu wrote:
On 01/29/2011 08:54 AM, Vincent Massol wrote:
No that's ok. I just wanted to make sure we all understand the limitation and the path
to the future.
Thanks
-Vincent
On Jan 29, 2011, at 3:59 AM, sdumitriu (SVN) wrote:
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. 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.
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();
5387
days inactive
5389
days old
2 comments
2 participants
participants (2)
-
Sergiu Dumitriu -
Vincent Massol