From 774f1c4cfeec607a8152f122f4186f4a21228e69 Mon Sep 17 00:00:00 2001 From: James Ahlborn Date: Tue, 18 Mar 2008 03:14:04 +0000 Subject: [PATCH] completely fix problems with sporadic usage map corruption; add some soft buffer caching in various places git-svn-id: https://svn.code.sf.net/p/jackcess/code/jackcess/trunk@281 f203690c-595d-4dc9-a70b-905162fa7fd2 --- src/changes/changes.xml | 7 + .../healthmarketscience/jackcess/Index.java | 23 ++- .../jackcess/NullMask.java | 9 +- .../jackcess/PageChannel.java | 15 +- .../healthmarketscience/jackcess/Table.java | 52 ++++-- .../jackcess/TempBufferHolder.java | 173 ++++++++++++++++++ .../jackcess/TempPageHolder.java | 96 ++-------- .../jackcess/UsageMap.java | 2 +- .../jackcess/TableTest.java | 3 +- 9 files changed, 266 insertions(+), 114 deletions(-) create mode 100644 src/java/com/healthmarketscience/jackcess/TempBufferHolder.java diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 4e76361..3180b13 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -31,6 +31,13 @@ Fix creation of tables with auto-number columns. + + Completely fix problems with sporadic usage map corruption. + + + Add some soft buffer caching in various places to avoid excessive + buffer reallocation. + diff --git a/src/java/com/healthmarketscience/jackcess/Index.java b/src/java/com/healthmarketscience/jackcess/Index.java index d52db99..df2445f 100644 --- a/src/java/com/healthmarketscience/jackcess/Index.java +++ b/src/java/com/healthmarketscience/jackcess/Index.java @@ -174,6 +174,9 @@ public class Index implements Comparable { private boolean _initialized; /** modification count for the table, keeps cursors up-to-date */ private int _modCount; + /** temp buffer used to writing the index */ + private final TempBufferHolder _indexBufferH = + TempBufferHolder.newHolder(false, true); /** FIXME, for now, we can't write multi-page indexes or indexes using the funky primary key compression scheme */ boolean _readOnly; @@ -315,7 +318,7 @@ public class Index implements Comparable { * Write this index out to a buffer */ private ByteBuffer write() throws IOException { - ByteBuffer buffer = getPageChannel().createPageBuffer(); + ByteBuffer buffer = _indexBufferH.getPageBuffer(getPageChannel()); buffer.put((byte) 0x04); //Page type buffer.put((byte) 0x01); //Unknown buffer.putShort((short) 0); //Free space @@ -492,7 +495,8 @@ public class Index implements Comparable { int lastStart = 0; byte[] valuePrefix = null; boolean firstEntry = true; - ByteBuffer tmpEntryBuffer = null; + TempBufferHolder tmpEntryBufferH = + TempBufferHolder.newHolder(true, true, ByteOrder.BIG_ENDIAN); for (int i = 0; i < entryMaskLength; i++) { byte entryMask = indexPage.get(entryMaskPos + i); @@ -506,9 +510,8 @@ public class Index implements Comparable { ByteBuffer curEntryBuffer = indexPage; int curEntryLen = length; if(valuePrefix != null) { - tmpEntryBuffer = getTempEntryBuffer( - indexPage, length, valuePrefix, tmpEntryBuffer); - curEntryBuffer = tmpEntryBuffer; + curEntryBuffer = getTempEntryBuffer( + indexPage, length, valuePrefix, tmpEntryBufferH); curEntryLen += valuePrefix.length; } @@ -543,14 +546,10 @@ public class Index implements Comparable { */ private ByteBuffer getTempEntryBuffer( ByteBuffer indexPage, int entryLen, byte[] valuePrefix, - ByteBuffer tmpEntryBuffer) + TempBufferHolder tmpEntryBufferH) { - int totalLen = entryLen + valuePrefix.length; - if((tmpEntryBuffer == null) || (tmpEntryBuffer.capacity() < totalLen)) { - tmpEntryBuffer = ByteBuffer.allocate(totalLen); - } else { - tmpEntryBuffer.clear(); - } + ByteBuffer tmpEntryBuffer = tmpEntryBufferH.getBuffer( + getPageChannel(), valuePrefix.length + entryLen); // combine valuePrefix and rest of entry from indexPage, then prep for // reading diff --git a/src/java/com/healthmarketscience/jackcess/NullMask.java b/src/java/com/healthmarketscience/jackcess/NullMask.java index a2d2625..dd5d959 100644 --- a/src/java/com/healthmarketscience/jackcess/NullMask.java +++ b/src/java/com/healthmarketscience/jackcess/NullMask.java @@ -56,9 +56,12 @@ public class NullMask { public void read(ByteBuffer buffer) { buffer.get(_mask); } - - public ByteBuffer wrap() { - return ByteBuffer.wrap(_mask); + + /** + * Write a mask to a buffer + */ + public void write(ByteBuffer buffer) { + buffer.put(_mask); } /** diff --git a/src/java/com/healthmarketscience/jackcess/PageChannel.java b/src/java/com/healthmarketscience/jackcess/PageChannel.java index 4aaa08b..b51e869 100644 --- a/src/java/com/healthmarketscience/jackcess/PageChannel.java +++ b/src/java/com/healthmarketscience/jackcess/PageChannel.java @@ -47,6 +47,8 @@ public class PageChannel implements Channel, Flushable { static final int INVALID_PAGE_NUMBER = -1; + static final ByteOrder DEFAULT_BYTE_ORDER = ByteOrder.LITTLE_ENDIAN; + /** dummy buffer used when allocating new pages */ private static final ByteBuffer FORCE_BYTES = ByteBuffer.allocate(1); @@ -107,10 +109,8 @@ public class PageChannel implements Channel, Flushable { /** * @param buffer Buffer to read the page into * @param pageNumber Number of the page to read in (starting at 0) - * @return True if the page was successfully read into the buffer, false if - * that page doesn't exist. */ - public boolean readPage(ByteBuffer buffer, int pageNumber) + public void readPage(ByteBuffer buffer, int pageNumber) throws IOException { if(pageNumber == INVALID_PAGE_NUMBER) { @@ -120,9 +120,14 @@ public class PageChannel implements Channel, Flushable { LOG.debug("Reading in page " + Integer.toHexString(pageNumber)); } buffer.clear(); - boolean rtn = _channel.read(buffer, (long) pageNumber * (long) getFormat().PAGE_SIZE) != -1; + int bytesRead = _channel.read( + buffer, (long) pageNumber * (long) getFormat().PAGE_SIZE); buffer.flip(); - return rtn; + if(bytesRead != getFormat().PAGE_SIZE) { + throw new IOException("Failed attempting to read " + + getFormat().PAGE_SIZE + " bytes from page " + + pageNumber + ", only read " + bytesRead); + } } /** diff --git a/src/java/com/healthmarketscience/jackcess/Table.java b/src/java/com/healthmarketscience/jackcess/Table.java index 73fe6d3..d850349 100644 --- a/src/java/com/healthmarketscience/jackcess/Table.java +++ b/src/java/com/healthmarketscience/jackcess/Table.java @@ -109,6 +109,15 @@ public class Table private UsageMap _freeSpacePages; /** modification count for the table, keeps row-states up-to-date */ private int _modCount; + /** page buffer used to update data pages when adding rows */ + private final TempPageHolder _addRowBufferH = + TempPageHolder.newHolder(false); + /** page buffer used to update the table def page */ + private final TempPageHolder _tableDefBufferH = + TempPageHolder.newHolder(false); + /** buffer used to writing single rows of data */ + private final TempBufferHolder _singleRowBufferH = + TempBufferHolder.newHolder(false, true); /** common cursor for iterating through the table, kept here for historic reasons */ @@ -1041,6 +1050,10 @@ public class Table // write the page data getPageChannel().writePage(pageBuffer, pageNumber); + // possibly invalidate the add row buffer if a different data buffer is + // being written (e.g. this happens during deleteRow) + _addRowBufferH.possiblyInvalidate(pageNumber, pageBuffer); + // update modification count so any active RowStates can keep themselves // up-to-date ++_modCount; @@ -1096,11 +1109,15 @@ public class Table * @param rows List of Object[] row values */ public void addRows(List rows) throws IOException { - ByteBuffer dataPage = getPageChannel().createPageBuffer(); ByteBuffer[] rowData = new ByteBuffer[rows.size()]; Iterator iter = rows.iterator(); for (int i = 0; iter.hasNext(); i++) { - rowData[i] = createRow(iter.next(), getFormat().MAX_ROW_SIZE); + // note, use the cached _singleRowBufferH for the first row. this will + // speed up single row writes + rowData[i] = createRow( + iter.next(), getFormat().MAX_ROW_SIZE, + ((i == 0) ? _singleRowBufferH.getPageBuffer(getPageChannel()) : + getPageChannel().createPageBuffer())); if (rowData[i].limit() > getFormat().MAX_ROW_SIZE) { throw new IOException("Row size " + rowData[i].limit() + " is too large"); @@ -1112,6 +1129,7 @@ public class Table // find last data page (Not bothering to check other pages for free // space.) + ByteBuffer dataPage = null; UsageMap.PageCursor revPageCursor = _ownedPages.cursor(); revPageCursor.afterLast(); while(true) { @@ -1119,7 +1137,7 @@ public class Table if(tmpPageNumber < 0) { break; } - getPageChannel().readPage(dataPage, tmpPageNumber); + dataPage = _addRowBufferH.setPage(getPageChannel(), tmpPageNumber); if(dataPage.get() == PageTypes.DATA) { // found last data page, only use if actually listed in free space // pages @@ -1132,7 +1150,8 @@ public class Table if(pageNumber == PageChannel.INVALID_PAGE_NUMBER) { // No data pages exist (with free space). Create a new one. - pageNumber = newDataPage(dataPage); + dataPage = newDataPage(); + pageNumber = _addRowBufferH.getPageNumber(); } for (int i = 0; i < rowData.length; i++) { @@ -1145,8 +1164,8 @@ public class Table writeDataPage(dataPage, pageNumber); _freeSpacePages.removePageNumber(pageNumber); - dataPage.clear(); - pageNumber = newDataPage(dataPage); + dataPage = newDataPage(); + pageNumber = _addRowBufferH.getPageNumber(); freeSpaceInPage = dataPage.getShort(getFormat().OFFSET_FREE_SPACE); } @@ -1172,8 +1191,8 @@ public class Table private void updateTableDefinition(int rowCountInc) throws IOException { // load table definition - ByteBuffer tdefPage = getPageChannel().createPageBuffer(); - getPageChannel().readPage(tdefPage, _tableDefPageNumber); + ByteBuffer tdefPage = _tableDefBufferH.setPage(getPageChannel(), + _tableDefPageNumber); // make sure rowcount and autonumber are up-to-date _rowCount += rowCountInc; @@ -1197,10 +1216,11 @@ public class Table * Create a new data page * @return Page number of the new page */ - private int newDataPage(ByteBuffer dataPage) throws IOException { + private ByteBuffer newDataPage() throws IOException { if (LOG.isDebugEnabled()) { LOG.debug("Creating new data page"); } + ByteBuffer dataPage = _addRowBufferH.setNewPage(getPageChannel()); dataPage.put(PageTypes.DATA); //Page type dataPage.put((byte) 1); //Unknown dataPage.putShort((short)getRowSpaceUsage(getFormat().MAX_ROW_SIZE, @@ -1208,17 +1228,19 @@ public class Table dataPage.putInt(_tableDefPageNumber); //Page pointer to table definition dataPage.putInt(0); //Unknown dataPage.putInt(0); //Number of records on this page - int pageNumber = getPageChannel().writeNewPage(dataPage); + int pageNumber = _addRowBufferH.getPageNumber(); + getPageChannel().writePage(dataPage, pageNumber); _ownedPages.addPageNumber(pageNumber); _freeSpacePages.addPageNumber(pageNumber); - return pageNumber; + return dataPage; } /** * Serialize a row of Objects into a byte buffer */ - ByteBuffer createRow(Object[] rowArray, int maxRowSize) throws IOException { - ByteBuffer buffer = getPageChannel().createPageBuffer(); + ByteBuffer createRow(Object[] rowArray, int maxRowSize, ByteBuffer buffer) + throws IOException + { buffer.putShort(_maxColumnCount); NullMask nullMask = new NullMask(_maxColumnCount); @@ -1318,8 +1340,8 @@ public class Table } buffer.putShort(_maxVarColumnCount); //Number of var length columns } - - buffer.put(nullMask.wrap()); //Null mask + + nullMask.write(buffer); //Null mask buffer.limit(buffer.position()); buffer.flip(); if (LOG.isDebugEnabled()) { diff --git a/src/java/com/healthmarketscience/jackcess/TempBufferHolder.java b/src/java/com/healthmarketscience/jackcess/TempBufferHolder.java new file mode 100644 index 0000000..49f510c --- /dev/null +++ b/src/java/com/healthmarketscience/jackcess/TempBufferHolder.java @@ -0,0 +1,173 @@ +// Copyright (c) 2008 Health Market Science, Inc. + +package com.healthmarketscience.jackcess; + +import java.lang.ref.PhantomReference; +import java.lang.ref.Reference; +import java.lang.ref.SoftReference; +import java.nio.ByteBuffer; +import java.nio.ByteOrder; + +/** + * Manages a reference to a ByteBuffer. + * + * @author James Ahlborn + */ +public abstract class TempBufferHolder { + + private static final Reference EMPTY_BUFFER_REF = + new SoftReference(null); + + /** whether or not every get automatically rewinds the buffer */ + private final boolean _autoRewind; + /** ByteOrder for all allocated buffers */ + private final ByteOrder _order; + /** the mod count of the current buffer (changes on every realloc) */ + private int _modCount; + + protected TempBufferHolder(boolean autoRewind, ByteOrder order) { + _autoRewind = autoRewind; + _order = order; + } + + /** + * @return the modification count of the current buffer (this count is + * changed every time the buffer is reallocated) + */ + public int getModCount() { + return _modCount; + } + + /** + * Creates a new TempBufferHolder. + * @param hard iff true, the TempBufferHolder will maintain a hard reference + * to the current buffer, otherwise will maintain a + * SoftReference. + * @param autoRewind whether or not every get automatically rewinds the + * buffer + */ + public static TempBufferHolder newHolder(boolean hard, boolean autoRewind) { + return newHolder(hard, autoRewind, PageChannel.DEFAULT_BYTE_ORDER); + } + + /** + * Creates a new TempBufferHolder. + * @param hard iff true, the TempBufferHolder will maintain a hard reference + * to the current buffer, otherwise will maintain a + * SoftReference. + * @param autoRewind whether or not every get automatically rewinds the + * buffer + * @param order byte order for all allocated buffers + */ + public static TempBufferHolder newHolder(boolean hard, boolean autoRewind, + ByteOrder order) + { + if(hard) { + return new HardTempBufferHolder(autoRewind, order); + } + return new SoftTempBufferHolder(autoRewind, order); + } + + /** + * Returns a ByteBuffer of at least the defined page size, with the limit + * set to the page size, and the predefined byteOrder. Will be rewound iff + * autoRewind is enabled for this buffer. + */ + public final ByteBuffer getPageBuffer(PageChannel pageChannel) { + return getBuffer(pageChannel, pageChannel.getFormat().PAGE_SIZE); + } + + /** + * Returns a ByteBuffer of at least the given size, with the limit set to + * the given size, and the predefined byteOrder. Will be rewound iff + * autoRewind is enabled for this buffer. + */ + public final ByteBuffer getBuffer(PageChannel pageChannel, int size) { + ByteBuffer buffer = getExistingBuffer(); + if((buffer == null) || (buffer.capacity() < size)) { + buffer = pageChannel.createBuffer(size, _order); + ++_modCount; + setNewBuffer(buffer); + } else { + buffer.limit(size); + } + if(_autoRewind) { + buffer.rewind(); + } + return buffer; + } + + /** + * @returns the currently referenced buffer, {@code null} if none + */ + public abstract ByteBuffer getExistingBuffer(); + + /** + * Releases any referenced memory. + */ + public abstract void clear(); + + /** + * Sets a new buffer for this holder. + */ + protected abstract void setNewBuffer(ByteBuffer newBuffer); + + /** + * TempBufferHolder which has a hard reference to the buffer buffer. + */ + private static final class HardTempBufferHolder extends TempBufferHolder + { + private ByteBuffer _buffer; + + private HardTempBufferHolder(boolean autoRewind, ByteOrder order) { + super(autoRewind, order); + } + + @Override + public ByteBuffer getExistingBuffer() { + return _buffer; + } + + @Override + protected void setNewBuffer(ByteBuffer newBuffer) { + _buffer = newBuffer; + } + + @Override + public void clear() { + _buffer = null; + } + } + + /** + * TempBufferHolder which has a soft reference to the buffer buffer. + */ + private static final class SoftTempBufferHolder extends TempBufferHolder + { + private Reference _buffer = EMPTY_BUFFER_REF; + + private SoftTempBufferHolder(boolean autoRewind, ByteOrder order) { + super(autoRewind, order); + } + + @Override + public ByteBuffer getExistingBuffer() { + return _buffer.get(); + } + + @Override + protected void setNewBuffer(ByteBuffer newBuffer) { + _buffer.clear(); + _buffer = new SoftReference(newBuffer); +// // FIXME, enable for testing (make this automatic) +// _buffer = new PhantomReference(newBuffer, null); + } + + @Override + public void clear() { + _buffer.clear(); + } + } + + +} diff --git a/src/java/com/healthmarketscience/jackcess/TempPageHolder.java b/src/java/com/healthmarketscience/jackcess/TempPageHolder.java index 04f35db..f1fbcd6 100644 --- a/src/java/com/healthmarketscience/jackcess/TempPageHolder.java +++ b/src/java/com/healthmarketscience/jackcess/TempPageHolder.java @@ -28,7 +28,6 @@ King of Prussia, PA 19406 package com.healthmarketscience.jackcess; import java.io.IOException; -import java.lang.ref.SoftReference; import java.nio.ByteBuffer; /** @@ -36,14 +35,18 @@ import java.nio.ByteBuffer; * * @author James Ahlborn */ -public abstract class TempPageHolder { - - private static final SoftReference EMPTY_BUFFER_REF = - new SoftReference(null); - - protected int _pageNumber = PageChannel.INVALID_PAGE_NUMBER; +public final class TempPageHolder { + + private int _pageNumber = PageChannel.INVALID_PAGE_NUMBER; + private final TempBufferHolder _buffer; + /** the last "modification" count of the buffer that this holder observed. + this is tracked so that the page data can be re-read if the underlying + buffer has been discarded since the last page read */ + private int _bufferModCount; - protected TempPageHolder() { + private TempPageHolder(boolean hard) { + _buffer = TempBufferHolder.newHolder(hard, false); + _bufferModCount = _buffer.getModCount(); } /** @@ -53,10 +56,7 @@ public abstract class TempPageHolder { * SoftReference. */ public static TempPageHolder newHolder(boolean hard) { - if(hard) { - return new HardTempPageHolder(); - } - return new SoftTempPageHolder(); + return new TempPageHolder(hard); } /** @@ -91,9 +91,11 @@ public abstract class TempPageHolder { boolean rewind) throws IOException { - ByteBuffer buffer = getBuffer(pageChannel); - if(pageNumber != _pageNumber) { + ByteBuffer buffer = _buffer.getPageBuffer(pageChannel); + int modCount = _buffer.getModCount(); + if((pageNumber != _pageNumber) || (_bufferModCount != modCount)) { _pageNumber = pageNumber; + _bufferModCount = modCount; pageChannel.readPage(buffer, _pageNumber); } else if(rewind) { buffer.rewind(); @@ -114,7 +116,7 @@ public abstract class TempPageHolder { // allocate a new page in the database _pageNumber = pageChannel.allocateNewPage(); // return a new buffer - return getBuffer(pageChannel); + return _buffer.getPageBuffer(pageChannel); } /** @@ -134,7 +136,7 @@ public abstract class TempPageHolder { */ public void possiblyInvalidate(int modifiedPageNumber, ByteBuffer modifiedBuffer) { - if(modifiedBuffer == getExistingBuffer()) { + if(modifiedBuffer == _buffer.getExistingBuffer()) { // no worries, our buffer was the one modified (or is null, either way // we'll need to reload) return; @@ -151,67 +153,7 @@ public abstract class TempPageHolder { */ public void clear() { invalidate(); + _buffer.clear(); } - protected abstract ByteBuffer getExistingBuffer(); - - protected abstract ByteBuffer getBuffer(PageChannel pageChannel); - - /** - * TempPageHolder which has a hard reference to the page buffer. - */ - private static class HardTempPageHolder extends TempPageHolder - { - private ByteBuffer _buffer; - - @Override - protected ByteBuffer getExistingBuffer() { - return _buffer; - } - - @Override - protected ByteBuffer getBuffer(PageChannel pageChannel) { - if(_buffer == null) { - _buffer = pageChannel.createPageBuffer(); - } - return _buffer; - } - - @Override - public void clear() { - super.clear(); - _buffer = null; - } - } - - /** - * TempPageHolder which has a soft reference to the page buffer. - */ - private static class SoftTempPageHolder extends TempPageHolder - { - private SoftReference _buffer = EMPTY_BUFFER_REF; - - @Override - protected ByteBuffer getExistingBuffer() { - return _buffer.get(); - } - - @Override - protected ByteBuffer getBuffer(PageChannel pageChannel) { - ByteBuffer buffer = _buffer.get(); - if(buffer == null) { - buffer = pageChannel.createPageBuffer(); - _buffer = new SoftReference(buffer); - } - return buffer; - } - - @Override - public void clear() { - super.clear(); - _buffer.clear(); - } - } - - } diff --git a/src/java/com/healthmarketscience/jackcess/UsageMap.java b/src/java/com/healthmarketscience/jackcess/UsageMap.java index 9484ffb..64656ba 100644 --- a/src/java/com/healthmarketscience/jackcess/UsageMap.java +++ b/src/java/com/healthmarketscience/jackcess/UsageMap.java @@ -711,7 +711,7 @@ public class UsageMap mapPageBuffer.putShort((short) 0); //Unknown int mapPageNum = _mapPageHolder.getPageNumber(); getTableBuffer().putInt(calculateMapPagePointerOffset(pageIndex), - mapPageNum); + mapPageNum); writeTable(); return mapPageBuffer; } diff --git a/test/src/java/com/healthmarketscience/jackcess/TableTest.java b/test/src/java/com/healthmarketscience/jackcess/TableTest.java index 84e8669..85b6bf7 100644 --- a/test/src/java/com/healthmarketscience/jackcess/TableTest.java +++ b/test/src/java/com/healthmarketscience/jackcess/TableTest.java @@ -66,7 +66,8 @@ public class TableTest extends TestCase { row[0] = new Short((short) 9); row[1] = "Tim"; row[2] = "McCune"; - ByteBuffer buffer = table.createRow(row, format.MAX_ROW_SIZE); + ByteBuffer buffer = table.createRow(row, format.MAX_ROW_SIZE, + pageChannel.createPageBuffer()); assertEquals((short) colCount, buffer.getShort()); assertEquals((short) 9, buffer.getShort()); assertEquals((byte) 'T', buffer.get()); -- 2.39.5