diff --git a/src/changes/changes.xml b/src/changes/changes.xml index cf8cbc2c4..69f8a0e78 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -239,6 +239,9 @@ Add JsonDataSet, JsonProducer, and JsonWriter for reading and writing JSON-format datasets, including Ant <export>/<operation>/<compare> task support via a new "json" format; null column values are omitted on write and resolved from missing keys on read. Update maven dependency com.fasterxml.jackson.core:jackson-databind from 2.18.2 to 2.18.9 (#914). + + Reduce FlatXmlProducer memory footprint: column names built from SAX attribute names, previously a fresh String per occurrence even when identical to one already seen elsewhere in the document, now flow through a per-parse cache so repeated column names across rows and tables share one String instance instead of each Column retaining its own duplicate copy. Attribute values are left untouched since they are far less likely to repeat. + diff --git a/src/main/java/org/dbunit/dataset/xml/FlatXmlProducer.java b/src/main/java/org/dbunit/dataset/xml/FlatXmlProducer.java index adbbf8ec7..2e61307ce 100644 --- a/src/main/java/org/dbunit/dataset/xml/FlatXmlProducer.java +++ b/src/main/java/org/dbunit/dataset/xml/FlatXmlProducer.java @@ -23,10 +23,12 @@ import java.io.IOException; import java.io.StringReader; import java.util.ArrayList; +import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Set; import javax.xml.parsers.ParserConfigurationException; @@ -120,7 +122,19 @@ public class FlatXmlProducer extends DefaultHandler implements IDataSetProducer, */ private Set _activeColumnNamesUpperCase; - + /** + * Canonical instances of column names built from SAX attribute names, keyed by + * themselves. Column names repeat across many rows of a table and often across + * different tables, but the SAX parser hands back a fresh String for each + * occurrence; routing every {@link Column} construction through this cache lets + * those repeats share one instance instead of each being retained separately for + * the life of the parsed dataset. Live only for the duration of a single + * {@link #produce()} call: (re)created at its start and cleared at its end, since + * the cache itself serves no purpose once the columns it produced are handed off + * to the consumer. + */ + private Map _columnNameCache; + /** * Creates a producer that reads the given XML source, with DTD metadata enabled. * @@ -237,13 +251,32 @@ private ITableMetaData createTableMetaData(String tableName, Attributes attribut Column[] columns = new Column[attributes.getLength()]; for (int i = 0; i < attributes.getLength(); i++) { - columns[i] = new Column(attributes.getQName(i), DataType.UNKNOWN); + String columnName = internColumnName(attributes.getQName(i)); + columns[i] = new Column(columnName, DataType.UNKNOWN); } return new DefaultTableMetaData(tableName, columns); } - - + + /** + * Returns a canonical String instance equal to the given column name, reusing a + * previously-cached instance when this parse has already seen that name. + * + * @param columnName The column name to canonicalize. + * @return A canonical String instance equal to columnName. + */ + private String internColumnName(String columnName) + { + String cachedName = _columnNameCache.get(columnName); + if (cachedName != null) + { + return cachedName; + } + + _columnNameCache.put(columnName, columnName); + return columnName; + } + /** * merges the existing columns with the potentially new ones. * @param columnsToMerge List of extra columns found, which need to be merge back into the metadata. @@ -345,7 +378,15 @@ protected void handleMissingColumns(Attributes attributes) { if (!_activeColumnNamesUpperCase.contains(attributes.getQName(i).toUpperCase(Locale.ENGLISH))) { - columnsToMerge.add(new Column(attributes.getQName(i), DataType.UNKNOWN)); + // Only canonicalize a name that will actually be retained in metadata: when + // column sensing is disabled, this Column is discarded right after the warning + // below is built, and caching its name would just grow the cache for nothing. + String columnName = attributes.getQName(i); + if (_columnSensing) + { + columnName = internColumnName(columnName); + } + columnsToMerge.add(new Column(columnName, DataType.UNKNOWN)); } } @@ -418,6 +459,8 @@ public void produce() throws DataSetException { logger.debug("produce() - start"); + _columnNameCache = new HashMap(); + try { SAXParserFactory saxParserFactory = SAXParserFactory.newInstance(); @@ -448,6 +491,12 @@ public void produce() throws DataSetException { throw new DataSetException(e); } + finally + { + // Only needed for the duration of a single parse; drop it immediately after + // instead of keeping it alive for as long as this producer instance is. + _columnNameCache = null; + } } //////////////////////////////////////////////////////////////////////////// diff --git a/src/test/java/org/dbunit/dataset/xml/FlatXmlProducerTest.java b/src/test/java/org/dbunit/dataset/xml/FlatXmlProducerTest.java index 7c468a860..53d568910 100644 --- a/src/test/java/org/dbunit/dataset/xml/FlatXmlProducerTest.java +++ b/src/test/java/org/dbunit/dataset/xml/FlatXmlProducerTest.java @@ -20,19 +20,24 @@ */ package org.dbunit.dataset.xml; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; import java.io.File; import java.io.IOException; import java.io.StringReader; +import java.util.ArrayList; +import java.util.List; import org.dbunit.TurkishDefaultLocale; import org.dbunit.dataset.Column; import org.dbunit.dataset.DataSetException; import org.dbunit.dataset.DefaultDataSet; import org.dbunit.dataset.DefaultTable; +import org.dbunit.dataset.ITableMetaData; import org.dbunit.dataset.datatype.DataType; import org.dbunit.dataset.stream.AbstractProducerTest; +import org.dbunit.dataset.stream.DefaultConsumer; import org.dbunit.dataset.stream.IDataSetProducer; import org.dbunit.dataset.stream.MockDataSetConsumer; import org.dbunit.testutil.TestUtils; @@ -305,4 +310,98 @@ void testProduce_turkishLocaleAndCaseVariantColumnName_recognizesSameColumn() th consumer.verify(); } + @Test + void testProduce_sameColumnNameAcrossTables_reusesColumnNameStringInstance() throws Exception + { + // Two distinct tables sharing a column name, and no DTD/metaDataSet, so both + // tables' metadata is built straight from SAX attribute names. + final String content = "" + "" + + "" + "" + + ""; + final InputSource source = new InputSource(new StringReader(content)); + final IDataSetProducer producer = new FlatXmlProducer(source); + + final List capturedMetaData = new ArrayList<>(); + producer.setConsumer(new DefaultConsumer() + { + @Override + public void startTable(final ITableMetaData metaData) throws DataSetException + { + capturedMetaData.add(metaData); + } + }); + + producer.produce(); + + final String tableAColumnName = + capturedMetaData.get(0).getColumns()[0].getColumnName(); + final String tableBColumnName = + capturedMetaData.get(1).getColumns()[0].getColumnName(); + + assertThat(tableBColumnName) + .as("FlatXmlProducer should reuse one column-name String instance " + + "across tables sharing a column name instead of allocating " + + "a duplicate copy per table.") + .isSameAs(tableAColumnName); + } + + @Test + void testProduce_columnSensedNameMatchesLaterTable_reusesColumnNameStringInstance() throws Exception + { + // TABLE_A senses COL_SHARED via handleMissingColumns, introduced only on its + // second row; TABLE_B declares COL_SHARED directly in its first row, so its + // metadata comes from createTableMetaData instead. Column sensing must be + // enabled, or TABLE_A's second row would just log a warning and drop COL_SHARED. + final String content = "" + "" + + "" + + "" + + "" + ""; + final InputSource source = new InputSource(new StringReader(content)); + final IDataSetProducer producer = + new FlatXmlProducer(source, false, true); + + final List capturedMetaData = new ArrayList<>(); + producer.setConsumer(new DefaultConsumer() + { + @Override + public void startTable(final ITableMetaData metaData) throws DataSetException + { + capturedMetaData.add(metaData); + } + }); + + producer.produce(); + + final ITableMetaData tableAMetaData = + findTableMetaData(capturedMetaData, "TABLE_A"); + final ITableMetaData tableBMetaData = + findTableMetaData(capturedMetaData, "TABLE_B"); + + final int sensedColumnIndex = tableAMetaData.getColumnIndex("COL_SHARED"); + final int declaredColumnIndex = tableBMetaData.getColumnIndex("COL_SHARED"); + final String sensedColumnName = + tableAMetaData.getColumns()[sensedColumnIndex].getColumnName(); + final String declaredColumnName = + tableBMetaData.getColumns()[declaredColumnIndex].getColumnName(); + + assertThat(sensedColumnName) + .as("FlatXmlProducer should reuse the same column-name String " + + "instance whether a column's metadata comes from column " + + "sensing or from a later table's first row.") + .isSameAs(declaredColumnName); + } + + private static ITableMetaData findTableMetaData( + final List capturedMetaData, final String tableName) + { + for (final ITableMetaData metaData : capturedMetaData) + { + if (metaData.getTableName().equals(tableName)) + { + return metaData; + } + } + throw new AssertionError("No captured metadata for table " + tableName + "."); + } + }