Bug 47403 - HSSFRow should be backed by a Linked List
Summary: HSSFRow should be backed by a Linked List
Status: RESOLVED WORKSFORME
Alias: None
Product: POI
Classification: Unclassified
Component: HSSF (show other bugs)
Version: 3.5-dev
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-06-22 14:28 UTC by Joe Pollard
Modified: 2009-06-23 13:50 UTC (History)
1 user (show)



Attachments
Cells is now backed by a linked list. (6.49 KB, patch)
2009-06-22 14:28 UTC, Joe Pollard
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joe Pollard 2009-06-22 14:28:18 UTC
Created attachment 23848 [details]
Cells is now backed by a linked list.

Currently HSSFRow uses an array that occasionally needs to be resized, and inserting a column in the middle is not as efficient as it could be.

Please see the attached patch.

I've also added a method insertCell to make it more convenient to insert into the middle of a row.
Comment 1 Josh Micich 2009-06-22 17:09:37 UTC
Thanks for the patch.  Adding the method insertCell() to HSSFRow is a good idea.

I'm not sure however that the proposed optimisation is appropriate, given that it seems to be motivated by a method that isn't present yet.  I did a quick performance test, running the existing 997 junits six times, with and without the change.  My results show a degradation of about 5% (times in seconds):

poi trunk   (8.781, 9.375, 8.453, 8.453, 8.484, 8.453)
with change (9.078, 9.156, 9.125, 9.203, 9.047, 9.094)

Hopefully the POI unit tests are a fair representation of general POI usage (they *should* be, because many were based on example code posted to bugzilla).  I guess HSSFRow was written assuming that client code would generally use random access to read the cells.


If you could re-do the patch just for HSSFRow.insertCell(), that is likely to get applied.  However the optimisation needs more justification/tweaking.
Comment 2 Joe Pollard 2009-06-23 13:50:01 UTC
This fix became no longer necessary after 46266 was patched.  Thanks for the consideration, I'm marking this as resolved, as it's not really an issue anymore.