You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@poi.apache.org by Javen O'Neal <on...@apache.org> on 2017/07/13 15:26:13 UTC
Re: svn commit: r1801806 - in /poi/trunk/src/ooxml:
java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java java/org/apache/poi/xssf/streaming/SheetDataWriter.java
testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java
I like the idea of using XML-specific startElement, endElement, and
writeAttribute functions so that the code doesn't get littered with angle
brackets, quotes, and escaping attributes that contain quotes, ampersands,
and other characters.
I think Java will convert "a"+"b"+"c" into
StringBuilder("a").append("b").append("c").toString() during the
compilation process, so we probably don't need to make those replacements
unless they improve readability.
On Jul 13, 2017 12:14 AM, <fa...@apache.org> wrote:
> Author: fanningpj
> Date: Thu Jul 13 07:14:01 2017
> New Revision: 1801806
>
> URL: http://svn.apache.org/viewvc?rev=1801806&view=rev
> Log:
> avoid unnecessary string concats in SXSSF SheetDataWriter
>
> Modified:
> poi/trunk/src/ooxml/java/org/apache/poi/xssf/streaming/
> SXSSFWorkbook.java
> poi/trunk/src/ooxml/java/org/apache/poi/xssf/streaming/
> SheetDataWriter.java
> poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/streaming/
> TestSXSSFWorkbook.java
>
> Modified: poi/trunk/src/ooxml/java/org/apache/poi/xssf/streaming/
> SXSSFWorkbook.java
> URL: http://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/
> apache/poi/xssf/streaming/SXSSFWorkbook.java?rev=
> 1801806&r1=1801805&r2=1801806&view=diff
> ============================================================
> ==================
> --- poi/trunk/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java
> (original)
> +++ poi/trunk/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java
> Thu Jul 13 07:14:01 2017
> @@ -406,7 +406,7 @@ public class SXSSFWorkbook implements Wo
> }
>
> private static void copyStreamAndInjectWorksheet(InputStream in,
> OutputStream out, InputStream worksheetData) throws IOException {
> - InputStreamReader inReader=new InputStreamReader(in,"UTF-8");
> //TODO: Is it always UTF-8 or do we need to read the xml encoding
> declaration in the file? If not, we should perhaps use a SAX reader instead.
> + InputStreamReader inReader=new InputStreamReader(in,"UTF-8");
> OutputStreamWriter outWriter=new OutputStreamWriter(out,"UTF-8");
> boolean needsStartTag = true;
> int c;
>
> Modified: poi/trunk/src/ooxml/java/org/apache/poi/xssf/streaming/
> SheetDataWriter.java
> URL: http://svn.apache.org/viewvc/poi/trunk/src/ooxml/java/org/
> apache/poi/xssf/streaming/SheetDataWriter.java?rev=
> 1801806&r1=1801805&r2=1801806&view=diff
> ============================================================
> ==================
> --- poi/trunk/src/ooxml/java/org/apache/poi/xssf/streaming/SheetDataWriter.java
> (original)
> +++ poi/trunk/src/ooxml/java/org/apache/poi/xssf/streaming/SheetDataWriter.java
> Thu Jul 13 07:14:01 2017
> @@ -207,23 +207,27 @@ public class SheetDataWriter implements
> }
>
> void beginRow(int rownum, SXSSFRow row) throws IOException {
> - _out.write("<row r=\"" + (rownum + 1) + "\"");
> - if (row.hasCustomHeight())
> - _out.write(" customHeight=\"true\" ht=\"" +
> row.getHeightInPoints() + "\"");
> - if (row.getZeroHeight())
> - _out.write(" hidden=\"true\"");
> + _out.write("<row");
> + writeAttribute("r", Integer.toString(rownum + 1));
> + if (row.hasCustomHeight()) {
> + writeAttribute("customHeight", "true");
> + writeAttribute("ht", Float.toString(row.
> getHeightInPoints()));
> + }
> + if (row.getZeroHeight()) {
> + writeAttribute("hidden", "true");
> + }
> if (row.isFormatted()) {
> - _out.write(" s=\"" + row.getRowStyleIndex() + "\"");
> - _out.write(" customFormat=\"1\"");
> + writeAttribute("s", Integer.toString(row.
> getRowStyleIndex()));
> + writeAttribute("customFormat", "1");
> }
> if (row.getOutlineLevel() != 0) {
> - _out.write(" outlineLevel=\"" + row.getOutlineLevel() + "\"");
> + writeAttribute("outlineLevel", Integer.toString(row.
> getOutlineLevel()));
> }
> if(row.getHidden() != null) {
> - _out.write(" hidden=\"" + (row.getHidden() ? "1" : "0") +
> "\"");
> + writeAttribute("hidden", row.getHidden() ? "1" : "0");
> }
> if(row.getCollapsed() != null) {
> - _out.write(" collapsed=\"" + (row.getCollapsed() ? "1" : "0")
> + "\"");
> + writeAttribute("collapsed", row.getCollapsed() ? "1" : "0");
> }
>
> _out.write(">\n");
> @@ -239,30 +243,32 @@ public class SheetDataWriter implements
> return;
> }
> String ref = new CellReference(_rownum,
> columnIndex).formatAsString();
> - _out.write("<c r=\"" + ref + "\"");
> + _out.write("<c");
> + writeAttribute("r", ref);
> CellStyle cellStyle = cell.getCellStyle();
> if (cellStyle.getIndex() != 0) {
> // need to convert the short to unsigned short as the indexes
> can be up to 64k
> // ideally we would use int for this index, but that would
> need changes to some more
> // APIs
> - _out.write(" s=\"" + (cellStyle.getIndex() & 0xffff) + "\"");
> + writeAttribute("s", Integer.toString(cellStyle.getIndex() &
> 0xffff));
> }
> CellType cellType = cell.getCellTypeEnum();
> switch (cellType) {
> case BLANK: {
> - _out.write(">");
> + _out.write('>');
> break;
> }
> case FORMULA: {
> - _out.write(">");
> - _out.write("<f>");
> + _out.write("><f>");
> outputQuotedString(cell.getCellFormula());
> _out.write("</f>");
> switch (cell.getCachedFormulaResultTypeEnum()) {
> case NUMERIC:
> double nval = cell.getNumericCellValue();
> if (!Double.isNaN(nval)) {
> - _out.write("<v>" + nval + "</v>");
> + _out.write("<v>");
> + _out.write(Double.toString(nval));
> + _out.write("</v>");
> }
> break;
> default:
> @@ -275,15 +281,15 @@ public class SheetDataWriter implements
> XSSFRichTextString rt = new XSSFRichTextString(cell.
> getStringCellValue());
> int sRef = _sharedStringSource.addEntry(
> rt.getCTRst());
>
> - _out.write(" t=\"" + STCellType.S + "\">");
> - _out.write("<v>");
> + writeAttribute("t", STCellType.S.toString());
> + _out.write("><v>");
> _out.write(String.valueOf(sRef));
> _out.write("</v>");
> } else {
> - _out.write(" t=\"inlineStr\">");
> - _out.write("<is><t");
> + writeAttribute("t", "inlineStr");
> + _out.write("><is><t");
> if (hasLeadingTrailingSpaces(cell.getStringCellValue()))
> {
> - _out.write(" xml:space=\"preserve\"");
> + writeAttribute("xml:space", "preserve");
> }
> _out.write(">");
> outputQuotedString(cell.getStringCellValue());
> @@ -292,20 +298,26 @@ public class SheetDataWriter implements
> break;
> }
> case NUMERIC: {
> - _out.write(" t=\"n\">");
> - _out.write("<v>" + cell.getNumericCellValue() + "</v>");
> + writeAttribute("t", "n");
> + _out.write("><v>");
> + _out.write(Double.toString(cell.getNumericCellValue()));
> + _out.write("</v>");
> break;
> }
> case BOOLEAN: {
> - _out.write(" t=\"b\">");
> - _out.write("<v>" + (cell.getBooleanCellValue() ? "1" :
> "0") + "</v>");
> + writeAttribute("t", "b");
> + _out.write("><v>");
> + _out.write(cell.getBooleanCellValue() ? "1" : "0");
> + _out.write("</v>");
> break;
> }
> case ERROR: {
> FormulaError error = FormulaError.forInt(cell.
> getErrorCellValue());
>
> - _out.write(" t=\"e\">");
> - _out.write("<v>" + error.getString() + "</v>");
> + writeAttribute("t", "e");
> + _out.write("><v>");
> + _out.write(error.getString());
> + _out.write("</v>");
> break;
> }
> default: {
> @@ -315,6 +327,13 @@ public class SheetDataWriter implements
> _out.write("</c>");
> }
>
> + private void writeAttribute(String name, String value) throws
> IOException {
> + _out.write(' ');
> + _out.write(name);
> + _out.write("=\"");
> + _out.write(value);
> + _out.write('\"');
> + }
>
> /**
> * @return whether the string has leading / trailing spaces that
>
> Modified: poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/streaming/
> TestSXSSFWorkbook.java
> URL: http://svn.apache.org/viewvc/poi/trunk/src/ooxml/testcases/
> org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java?rev=
> 1801806&r1=1801805&r2=1801806&view=diff
> ============================================================
> ==================
> --- poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java
> (original)
> +++ poi/trunk/src/ooxml/testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java
> Thu Jul 13 07:14:01 2017
> @@ -132,7 +132,7 @@ public final class TestSXSSFWorkbook ext
> public void useSharedStringsTable() throws Exception {
> SXSSFWorkbook wb = new SXSSFWorkbook(null, 10, false, true);
>
> - SharedStringsTable sss = POITestCase.getFieldValue(SXSSFWorkbook.class,
> wb, SharedStringsTable.class, "_sharedStringSource");
> + SharedStringsTable sss = POITestCase.getFieldValue(SXSSFWorkbook.class,
> wb, SharedStringsTable.class, "_sharedStringSource");
>
> assertNotNull(sss);
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commits-unsubscribe@poi.apache.org
> For additional commands, e-mail: commits-help@poi.apache.org
>
>
Re: svn commit: r1801806 - in /poi/trunk/src/ooxml:
java/org/apache/poi/xssf/streaming/SXSSFWorkbook.java
java/org/apache/poi/xssf/streaming/SheetDataWriter.java
testcases/org/apache/poi/xssf/streaming/TestSXSSFWorkbook.java
Posted by "pj.fanning" <fa...@yahoo.com>.
I thought that the writeAttribute made the code a bit easier to read.
Also, concatenating strings before writing them to the Writer does add
overhead.
I did some similar work (with others) on a similar writer implementation in
February and removing the string concats and using additional write calls
instead did lead to better performance.
These changes were in:
https://github.com/prometheus/client_java/commits/master/simpleclient_common/src/main/java/io/prometheus/client/exporter/common/TextFormat.java
The performance test: https://github.com/pkubowicz/prometheus-client-test
I'm not wedded to the r1801806 change, so if there are objections, I can
revert it.
--
View this message in context: http://apache-poi.1045710.n5.nabble.com/Re-svn-commit-r1801806-in-poi-trunk-src-ooxml-java-org-apache-poi-xssf-streaming-SXSSFWorkbook-java-a-tp5728214p5728215.html
Sent from the POI - Dev mailing list archive at Nabble.com.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org