Bug 58909

Summary: [PATCH] Performance problem on cloneSheet/setSheetName
Product: POI Reporter: Luca Martini <lucamartini>
Component: XSSFAssignee: POI Developers List <dev>
Severity: enhancement CC: lucamartini
Priority: P2 Keywords: PatchAvailable
Version: 3.13-FINAL   
Target Milestone: ---   
Hardware: All   
OS: All   
Attachments: proposed patch
proposed patch in tar.gz.format

Description Luca Martini 2016-01-22 10:38:42 UTC
Created attachment 33476 [details]
proposed patch

Hi all,
I noticed a performance problem using the cloneSheet method of the XSSFWorbook class.
Currently, the cloneSheet generates an unique name for the cloned sheet.
However, it is common that the generated name has to be changed later to something more meaningful.
So, a typical use is the following:

XSSFSheet cloned = wb.cloneSheet(idx);
int origCopyIndex = wb.getSheetIndex(cloned);
wb.setSheetName(origCopyIndex, "OtherName");

In our application we use this pattern and we have noticed a huge performance hit in the setSheetName method because the setSheetName triggers the check of all the formulas in the workbook to update references to the sheet whose name was changed. When the workbook has a lot of cells this operation can last a significant amount of time.

However, this operation on a cloned sheet is completely not necessary because the generated name won't be in any formula in the workbook.

What I suggest is to add another version of the clonedSheet method for creating a cloned version with a given name. In this way, it is possible to avoid the setSheetName.

Something like:

  * Create an XSSFSheet from an existing sheet in the XSSFWorkbook.
  *  The cloned sheet is a deep copy of the original but with a new given
  *  name.
  * @return XSSFSheet representing the cloned sheet.
  * @throws IllegalArgumentException if the sheet index or the sheet
  *         name is invalid
  * @throws POIXMLException if there were errors when cloning
  public XSSFSheet cloneSheet(int sheetNum, String newName)

I attach a patch that includes the newly added method on the XSSFWorkbook class and some unit tests. I already checked that all existing tests still succeed after the application of the patch.

You can evaluate if this method should be pull up to the Workbook interface (in this case it is necessary to provide implementations for the other formats).
Comment 1 Luca Martini 2016-01-22 10:55:57 UTC
Created attachment 33477 [details]
proposed patch in tar.gz.format

I'm sorry but in my previous comment I uploaded a patch in the Eclipse format. This new attachment is in the requested format (generated by the patch.xml ant script).
Comment 2 Dominik Stadler 2016-03-31 13:33:37 UTC
This was applied via r1737237 in time for the upcoming 3.15-beta1. For now only XSSF has this method. I have adjusted and combined the unit-tests somewhat. 

Thanks for the Patch!
Comment 3 Javen O'Neal 2016-03-31 16:14:05 UTC
Either patch format (or github pull request) is acceptable.