Using fpspreadsheet to write huge xlsx files
lazarus / 49-FPSpreadsheet

----------------------------------------------------------------------
wp · 2014-07-10 21:19

There has been discussion that fpspreadsheet is not suited for large
data files. Since the entire spreadsheet grid is held in memory all
the time this is certainly true.

In order to overcome this limitation, a "virtual mode" has been
introduced in svn revision 3306 of the package. "Virtual", in this
context, is to be understood in the same way as in "virtual listbox"
or "virtual treeview". It means that writing of a spreadsheet file
does not require permanently available data stored in a worksheet, but
gets the data temporarily by means of an event. Therefore, in
principle, the file written can be of any size. A limitation, of
course, is given by the temporary memory streams used for collecting
data. But there is an option to avoid this limitation by using file
streams.

The main application, I guess, is exporting large database files to
spreadsheet formats.

Here is a short "getting started":
 Activate "woVirtualMode" of the worksheet's new "WritingOptions".  If
you expect "many" rows overflowing the available memory activate also
the option "woSaveMemory" which triggers usage of TFileStream for
temporary storage. Using the worksheet's properties "VirtualRowCount"
and "VirtualColCount" define how many data will be saved. Before
calling the "WriteToFile" method of the spreadsheet open the dataset
to be exported. Write an event handler for the event "OnNeedCellData"
of the worksheet. This event is called for every cell within the range
defined by VirtualRowCount and VirtualColCount and expects the value
to be stored for this cell. The loop progresses first along the rows,
then along the columns. In database terms, the columns correspond the
the fields of a dataset, the rows correspond to the records. Please
note that you are responsible for advancing the dataset cursor if the
last field of a record (the last column of a row) has been handled.
Call "WriteToFile" Of course, you must be aware that writing of huge
files takes its time. You can use the OnNeedCellData event to provide
a progress display.There is a little (non-database) demo
("test_virtualmode") in the folder "examples/other" of the
fpspreadsheet installtion.

In my tests, the Windows task manager does not show any increase of
memory usage in this mode.

Currently this feature is only available for xlsx files, but the
writers of other spreadsheet format will be upgraded shortly.

----------------------------------------------------------------------
Mike.Cornflake · 2014-07-11 06:14

I am very impressed, both with your response time and your dedication
to fpspreadsheet.

Quick question (for interest only):  I presume it's only the content
that you're immediately writing out?  I assume styles would still be
cached and written out last?

Another question (again for interest only):  Do you handle exporting
images in fpspreadsheet?

Actually both of these questions have a hidden agenda :-):  When I get
time (likely Sept) I now fully intend to rewrite the docx writer in
line with the lessons you've learned over the past few days. 
Everything you've gone through is applicable there as well.

----------------------------------------------------------------------
wp · 2014-07-11 08:15

Quote I presume it's only the content that you're immediately writing
out?  I assume styles would still be cached and written out last?The
present version only writes the content. Styles, in spreadsheet files,
are written first and referred to by an index when the cell is
written. If that feature should become important I think I could use
the styles of some template cells in the worksheet. But this is not on
my list so far.

QuoteDo you handle exporting images in fpspreadsheet?No.

----------------------------------------------------------------------
Mike.Cornflake · 2014-07-11 09:52

Quote from: wp on July 11, 2014, 10:15:51 amStyles, in spreadsheet
files, are written first and referred to by an index when the cell is
written. If that feature should become important I think I could use
the styles of some template cells in the worksheet. But this is not on
my list so far.
Many thanks for the info.

----------------------------------------------------------------------
rvk · 2014-07-11 10:55

Wow, you've been busy   Great work.

So using woVirtualMode makes TsWorkbook just a passthrough for the
TsCustomSpreadWriter.
And using woSaveMemory makes the TsCustomSpreadWriter write to
temporary files (instead of memory).
It works perfectly. No memory usage at all with 20.000x100 cells.

When you're going to implement this in sfOpenDocument and sfExcel8 the
OnNeedCellData would also need a formatting parameter. The .GetCell
and ^.UsedFormattingFields wouldn't work anymore because the cells are
already written to disc. But if there is a parameter (for
OnNeedCellData) with a complete format-structure this can be used to
fill styles.xml directly.

My first test:
Code: [Select]Running: Building TsWorkbook and Writing sfOOXML
  All strings with Windows 7 pro and SSD drive
----------
Rows x Cols WritingOptions                Build  Write
10.000x100  [                           ]   0,7    5,0
10.000x100  [woVirtualMode              ]   0,0    7,2
10.000x100  [               woSaveMemory]   0,6   11,9
10.000x100  [woVirtualMode, woSaveMemory]   0,0   12,9
----------
20.000x100  [                           ]   1,3   10,1
20.000x100  [woVirtualMode              ]   0,0   14,6
20.000x100  [               woSaveMemory]   1,3   23,6
20.000x100  [woVirtualMode, woSaveMemory]   0,0   25,6
----------
30.000x100  [                           ]   2,0   15,3
30.000x100  [woVirtualMode              ]   0,0   21,9
30.000x100  [               woSaveMemory]   2,0   35,1
30.000x100  [woVirtualMode, woSaveMemory]   0,0   38,1
----------
40.000x100  [                           ]   2,7   20,5
40.000x100  [woVirtualMode              ]   0,0   29,8
40.000x100  [               woSaveMemory]   2,7   46,0
40.000x100  [woVirtualMode, woSaveMemory]   0,0   50,6
Ready

At this time i keep getting an error during more testing with a temp-
file creation in zipper but couldn't yet pinpoint why.
<< Unable to create file ".tmp". >>  Somewhere the FFilename in zipper
is empty and it throws an error. I'll report back if i found
something.

I'm also looking for a memory leak. Maybe they're connected

----------------------------------------------------------------------
rvk · 2014-07-11 12:20

I found two "problems" with the new xlsxooxml.pas.

1) With the xlsxooxml.pas/fpszipper.pp combination.

In fpszipper you have a constant:
Code: [Select]   DefaultInMemSize = 256*1024; { Files larger than 256k
are processed on disk   }
(It's assigned to FInMemSize in .Create)
When a file is larger than 256KB it is processed on disc.
But according to this code (starting at line 1392, fspzipper.pp
rev.3306):
Code: [Select]  Try
    StartZipFile(Item);
    If (FInfile.Size<=FInMemSize) then
      ZipStream:=TMemoryStream.Create
    else
      begin
      TmpFileName:=ChangeFileExt(FFileName,'.tmp');
      ZipStream:=TFileStream.Create(TmpFileName,fmCreate);
      end;
it uses FFileName for a temporary filename.

But in xlsxooxml.pas there isn't a Filename set for TZip in
WriteToStream. So creating large spreadsheet will always crash.
(I added FZip.FileName := 'temp___'; at line 618 and it was solved)

Actually (fps)zipper should check if FileName is set and if it's not
it should assign something to it before continuing (or give a proper
exception).

2) I found that the files created with woVirtualMode are a few KB
larger.
It seems that sheet1.xml are different for both methods.
I see in one sheet1.xml you do a <row r="19" spans="1:10000"> and in
the other <row r="19" spans="1:100">
In xlsxooxml.pas you set it to Rowcount for woVirtualMode and to
Colcount for [].
Typo ??? 


My test-run:
Code: [Select]Running: Building TsWorkbook and Writing sfOOXML
  All strings with Windows 7 pro and SSD drive
----------
Rows x Cols WritingOptions                Build  Write
10.000x100  [                           ]   1,6    5,1
10.000x100  [woVirtualMode              ]   0,0    7,2
10.000x100  [               woSaveMemory]   1,6   12,1
10.000x100  [woVirtualMode, woSaveMemory]   0,0   15,5
----------
20.000x100  [                           ]   3,3   10,5
20.000x100  [woVirtualMode              ]   0,0   14,6
20.000x100  [               woSaveMemory]   3,2   23,9
20.000x100  [woVirtualMode, woSaveMemory]   0,0   30,0
----------
30.000x100  [                           ]   4,9   15,6
30.000x100  [woVirtualMode              ]   0,0   21,7
30.000x100  [               woSaveMemory]   4,8   35,3
30.000x100  [woVirtualMode, woSaveMemory]   0,0   44,6
----------
40.000x100  [                           ]   6,5   20,8
40.000x100  [woVirtualMode              ]   0,0   29,0
40.000x100  [               woSaveMemory]   6,5   46,5
40.000x100  [woVirtualMode, woSaveMemory]   0,0   59,0
----------
Ready

(should i learn how to write bugreports in bugtracker )

----------------------------------------------------------------------
BigChimp · 2014-07-11 12:39

Quote from: rvk on July 11, 2014, 02:20:32 pm(should i learn how to
write bugreports in bugtracker )
AFAIC, yes - looks like a good idea if you want to patch tzipper
(Fpspreadsheet: up to wp - he's the one working on it these days)

----------------------------------------------------------------------
marcov · 2014-07-11 12:46

Quote from: rvk on July 11, 2014, 02:20:32 pm(should i learn how to
write bugreports in bugtracker )

As Reinier says, in general yes. Now no, since it is having problems
:-)

----------------------------------------------------------------------
BigChimp · 2014-07-11 12:46

Bugtracker works fine again here...

----------------------------------------------------------------------
wp · 2014-07-11 12:52

Quote the OnNeedCellData would also need a formatting parameterSo
you're saying that you want to format the data in the spreadsheet as
well? As I wrote in the other posting, my idea is to provide template
cells in the worksheet for each style requested and pass their style
index to an extended OnNeedCellData event.

However, it is a bit difficult to find out this index from outside the
writer. The writing process of fpspreadsheet is pretty much
encapsulated, there are only a few hooks into the outside world.

It would be another possibility to pass the row/col coordinates of the
template cell to the OnNeedCellData event, but then the writer would
have to seek for the style index again and again for each of the
millions of cells. Since the format list will be short I guess this
would not cost too much, but it's certainly better to sit down and
look for a better solution.

Quote i keep getting an error during more testing with a temp-file
creation in zipperI am seeing such errors also in BigChimp's unit
tests which have a lot of temp file access without the zipper being
involved. Therefore, I'd blame the fast consecutive deletion and
recreation of temporary files for this issue instead. No idea on the
mechanism, though.

I am also observing that sometimes the temporary files are not deleted
(they are in %(APPDATA)\Local\Temp and have a name beginning with
"fps" and 2-3 letters indicating the data type, like STY for
"styles"). The files are deleted by calling "DeleteFile" which exits
without an error, but the files are still there. ?!

QuoteBugtracker ... up to wpReports on bugs are always welcome...

----------------------------------------------------------------------
rvk · 2014-07-11 15:24

Quote from: wp on July 11, 2014, 02:52:51 pmQuote the OnNeedCellData
would also need a formatting parameterSo you're saying that you want
to format the data in the spreadsheet as well?
I'll take it these changes are going to be implemented for
fpsopendocument.pas as well? And at this moment there already is the
possibility to do some formatting like stated in the examples:
Code: [Select]  MyWorksheet.WriteUTF8Text(3, 3, '[N,E]');// D4
  MyCell := MyWorksheet.GetCell(3, 3);
  MyCell^.Border := [cbNorth, cbEast];
  MyCell^.UsedFormattingFields := [uffBorder];
If you're going to implement woVirtualMode there, i imagine this would
not be possible anymore (because everything is already passed through
to the writer-class to a xml-stream).
(And i don't expect you're going to implement the GetCell-function
when woVirtualMode is used, are you?)

To keep these formatting options in fpsopendocument.pas you could just
add a parameter (besides AData) for format (i.e. AFormat) with a
structure with formatting options to OnNeedCellData. You can apply
them directly in fpsopendocument.pas to the cell being written (or at
least build those format_lists and point the cell being written to
that index). (or is this not possible?). When this structure is in
place it could be later implemented for xlsxooxml.pas too (but that's
not on your list yet).

I'm not sure what you meant with "template cell" but if the AData-
parameter of OnNeedCellData could even be a TCell structure (without
the Row/Count) and you would have everything you'll need to fill the
cell (including formatting options). In WriteWorksheet you loop
through VirtualRowCount and VirtualColCount and receive this structure
(from NeedCellData) (that's why Row/Count are not needed, they are
forced by the caller) and it would be easy to build the styles-list.
Although this style list would need to be in memory because otherwise
you would need one style per cell. Keeping it in memory may not need
as much space because duplicate styles will be reused (as it is now).
The user would have to know what formatting to apply during the
NeedCellData but i don't see that as a problem.

Maybe i'm seeing that completely wrong but if you want to keep
formatting with woVirtualMode you'll have to sacrifice some memory
(which would be much if it's the same for a lot of cells) and if
you're not using formatting the styles_lists would be empty anyway.

QuoteI am seeing such errors also in BigChimp's unit tests which have
a lot of temp file access without the zipper being involved.
Therefore, I'd blame the fast consecutive deletion and recreation of
temporary files for this issue instead. No idea on the mechanism,
though.Ok, so this is a bug in fpszipper.pp (it is a bug because this
should be handled, maybe with adding delays when the file is
inaccessible or switching to another file).
It's weird, though, that after i added a FZip.Filename := in
xlsxooxml.pas the error went away.
(Maybe Windows doesn't like rapid creation and deletion of files
without a name but only extension i.e. ".tmp" which is what happens if
FFilename is not set)

----------------------------------------------------------------------
wp · 2014-07-11 16:31

QuoteI'll take it these changes are going to be implemented for
fpsopendocument.pas as well?Surely, but ods is a bit crazy because
there are no row/col indexes. The entire rectangle spanned from the
first row/col to the last row/col has to be filled with an occupied
cell or a placeholder telling that the cell is empty. Therefore, there
are a few cases to consider and this is the reason why it is not done
yet.

Quotethis would not be possible anymoreNo. In virtual mode, there are
no "cells" because they would occupy memory in the cell grid.

QuoteMyWorksheet.GetCell(3, 3)You should not call GetCell because it
creates a cell, i.e. occupies memory. The correct function would be
"FindCell". But as I said, in virtual mode there it won't find
anything.

Your idea with the format list: Certainly, but we don't have it - it
is as simple as that. fpspreadsheet stores formats in the cells, not
in a list (a temporary format list is created during reading and
writing, but you don't know the format index of a given cell in the
first place).  It is high priority on my to-do list, but it will break
existing code (cell^.NumberFormat := ... won't work any more!). Before
doing this step I want to complete fpspreadsheet within the existing
framework, in particular bring ooxml to the same level of compleness
of the other formats and maybe add some more formula-stuff. After that
we can discuss how to continue - I'd vote for a major rewrite making
cells to classes instead of records and trying to have least memory
attached to cells. Probably this will be a different incompatible
package (such as "fpspreadsheet_new").

Quoteif you want to keep formatting with woVirtualMode you'll have to
sacrifice some memoryI don't think so - I'll have to find a place
within the writing process where the formatting information is
completely known and an event handler can be hooked in.

BTW - and very important: I think formatting in virtual mode should be
is a constant format along each column, a different format for one or
a few header rows would be acceptable. But is will not be possible to
assign an arbitrary format to an arbitrary virtual cell. The basic
idea is that of database export: You have a table with n fields and m
records. A field corresponds to a column, a record to a row. All cells
of the same field, i.e. along a given column, should have the same
format. Except, maybe, for a header row.

----------------------------------------------------------------------
rvk · 2014-07-11 17:25

Quote from: wp on July 11, 2014, 06:31:30 pmYour idea with the format
list: Certainly, but we don't have it - it is as simple as that.
fpspreadsheet stores formats in the cells, not in a list (a temporary
format list is created during reading and writing, but you don't know
the format index of a given cell in the first place).Yeah... i don't
think i'm explaining it clear enough. (Is FFormattingStyles not a
format-list at this point? Which you use to search for same formatting
before assigning a style-index to a cell during writing?)

From the source:
Quote{@@
  Checks if the formatting style of a cell is in the list of manually
added
  FFormattingStyles and returns its index, or -1 if it isn't
  @param  AFormat  Cell containing the formatting styles which are
seeked in the FFormattingStyles array.
}
function TsCustomSpreadWriter.FindFormattingInList(AFormat: PCell):
Integer;      

Now with woVirtualMode your looping through VirtualRowCount and
VirtualColCount so every cell is received (empty or not) via the
NeedCellData. I'm just saying if we've got the possibility of also
giving an AFormat (at that point) you could check if that style is
already used (get the correct index from style-memory like
FFormattingStyles is now). If so write that style-index together with
the cell to the sheet-stream. If it is not used, store it in memory
(i.e. FFormattingStyles) for later reference and use that new index
for the cell. (The style.xml-stream could also be appended directly
because we have the FColumnStyleList in memory).

That way every cell can have it's own formatting (if every cell has
different formatting it would take memory but if lots of cells have
the same formatting the memory requirement will be low, it's up to the
user if he wants to apply lots of different styles).

That's just how i see it now... Don't worry if i'm explaining this not
clear enough.
In that case i will see how the fpsopendocument.pas evolves and get
back when i can be somewhat more concrete (with code-examples).

QuoteI'd vote for a major rewrite making cells to classes instead of
records and trying to have least memory attached to cells.Yeah... that
was also my thought... Make the properties of the TCell a sort of
TList or something so when properties aren't used they don't take any
space.

QuoteQuoteif you want to keep formatting with woVirtualMode you'll
have to sacrifice some memoryI don't think so - I'll have to find a
place within the writing process where the formatting information is
completely known and an event handler can be hooked in.That would
almost always mean two passes or like i suggested above collecting the
formatting information in NeedCellData and storing the unique styles
in memory (which should not take that much memory).

QuoteBut is will not be possible to assign an arbitrary format to an
arbitrary virtual cell.
... Except, maybe, for a header row.
That would be a shame because it would mean less control over the
overall design of your sheets.
(and how about a footer row)

Not sure if you noticed my earlier post about the xlsxooxml.pas typo
in <span> (so i include it here again):
Quote from: rvk on July 11, 2014, 02:20:32 pm2) I found that the files
created with woVirtualMode are a few KB larger.
It seems that sheet1.xml are different for both methods.
I see in one sheet1.xml you do a <row r="19" spans="1:10000"> and in
the other <row r="19" spans="1:100">
In xlsxooxml.pas you set it to Rowcount for woVirtualMode and to
Colcount for [].
Typo ???

----------------------------------------------------------------------
wp · 2014-07-11 21:22

QuoteNow with woVirtualMode your looping through VirtualRowCount and
VirtualColCount so every cell is received (empty or not) via the
NeedCellData. I'm just saying if we've got the possibility of also
giving an AFormat (at that point) you could check if that style is
already used (get the correct index from style-memory like
FFormattingStyles is now). If so write that style-index together with
the cell to the sheet-stream. If it is not used, store it in memory
(i.e. FFormattingStyles) for later reference and use that new index
for the cell. (The style.xml-stream could also be appended directly
because we have the FColumnStyleList in memory).
Let me repeat this: you want to extend the OnNeedCellData by a Format
parameter which is a cell:
Code: [Select]type
  TNeedCellDataEvent = procedure(Sender: TObject; ARow,ACol:Cardinal;
out AData: variant;
    out AFormat: TCell) of object;
Then in the event handler you pass data and format to the writer.

The problem is that the format must be found in the FormattingStyles
array to get its index. If not found you say that it could be added to
the array. But this is not possible because this styles list already
has been written at this time. In OOXML, that's not such a severe
problem because the styles are in a separate file which could be
written later, but in BIFF it is severe since this is a binary file
with many internal pointers. I definitely will not patch this file.

No, the way to go must be to define all formatting styles before
writing and then hook into the writing process once the formatting
styles have been collected by the writer's ListAllFormattingStyles.
When this is completed an event must fire, call it
"OnFormattingStylesCollected". In its handler you can seek the format
templates and determine their style index. Store these indexes
somewhere. Later, during writing the cells, the "OnNeedCellData" event
is fired, and you decide on the basis of the provided row and column
index which style index should be used. This makes sure that only
existing formats are used.

Before implementing this, however, I'd prefer to interrupt this
activity for the moment and add basic formatting to xlsooxml and
activate virtual mode for ods first. Otherwise there would be only
biff for testing, and I know that there are some subtle differences.
So I have to ask you for your patience...

I did indeed miss your other posting of this afternoon. The "spans"
issue was a typo as you suggested, it is fixed. There was another typo
with a missing closing xml bracket which would have crashed writing of
empty cells. I'll look at the filename issue of the zipper later.

@BigChimp, if you are reading this: I did not go into the zipper in
detail so far. But the header notes of the fpszipper say that
fpszipper should be replaced by the fpc zipper after FPC 2.7.1,
probably already after 2.6.4. Should the version in the "$IF" be
updated? And will the fpc zipper have the same issue that rvk reports?

----------------------------------------------------------------------
rvk · 2014-07-11 22:09

Quote from: wp on July 11, 2014, 11:22:37 pmyou want to extend the
OnNeedCellData by a Format parameter which is a cell
....
If not found you say that it could be added to the array. But this is
not possible because this styles list already has been written at this
time.
Yep. That's how i saw the call to OnNeedCellData .
But i didn't mean "added to the array" (because that stream is already
written) but (also) added to a small new memory-structure with unique
styles. For Opendocument and OOXML this shouldn't be a problem but i'm
not very familiar with the biff-format so i can't know if that a
problem there.

The way you described (with first collecting the styles) would be
basically the same (for .xlsx and .ods) because your looping though
the cells row by column so all styles would be gathered sequentially
and are directly indexed in memory for later and current cells. (But i
can't speak for the biff-format). I'll wait patiently on your
implementation and see how it compares to my vision

Don't worry about the time-frame. It's not a critical point at my end.
(so take your time which you should always do)

----------------------------------------------------------------------
vfclists · 2014-07-11 22:31

Does FPSpreadsheet have its own built-in expression evaluator, or does
it just enter the values and  formulae for the calculations to be done
by Excel, LibreOffice etc?

I asked a related question
http://forum.lazarus.freepascal.org/index.php/topic,25154.msg152320
and wonder if it has an expression evaluator that can be used
independently.

----------------------------------------------------------------------
wp · 2014-07-11 22:56

No, there is no expression evaluator in fpspreadsheet. Not sure yet if
I'll dare to write one...

What do you want it for? Write your own spreadsheets with formulas?
Then you could compose a formula as an RPN formula - don't worry, it
is easier as it sounds... The basics are explained in the wiki
(http://wiki.lazarus.freepascal.org/FPSpreadsheet#RPN_Formulas). This
is working for writing and reading, the rpn formulas can even be
calculated. Advantage: available only for the old xls files.

I answered your other posting with a link to a tutorial for using
"fpexprpars". This can be used independently and is very flexible.
There is also TFPMathExpressionBridge which provides a wrapper around
"fpexprpars". Or you could use "symbolic" which comes also along with
fpc. Just look for this keyword here in the forum, here have been some
related discussions.

----------------------------------------------------------------------
vfclists · 2014-07-12 08:12

I was hoping for something well used by Lazarus developers.

I have decided on ArtFormula for the time being which the author
announced here
http://forum.lazarus.freepascal.org/index.php?topic=18003.0

----------------------------------------------------------------------
wp · 2014-07-14 15:19

After giving up the idea with the style index formatting of virtual
cells becomes very easy: Just format some cells in the worksheet as a
template as needed in virtual mode. In the example provided with
fpspreadsheet ("other/test_virtualmode"), for example, all cells in
the first row have a bold type face and a gray background; this is
achieved by

Code: [Select]var
  templateCell: PCell;
 ...
  worksheet.WriteFontStyle(0, 0, [fssBold]);
  worksheet.WriteBackgroundColor(0, 0, scGray);
  templateCell := worksheet.FindCell(0, 0);  // you can also call
GetCell here

The event OnNeedData now has an additional parameter "AStyleCell" in
which you can specify the format of the currently saved cell:

Code: [Select]procedure TSomething.NeedCellData(Sender: TObject;
ARow,ACol: Cardinal;
  var AValue: variant; var AStyleCell: PCell);
begin
  if ARow = 0 then begin
    AStyleCell := templateCell;
  ....

It is important that the style cell belongs to the workbook being
saved because the writer collects all styles contained in the workbook
before writing. So, please don't create style cells on the fly within
the NeedCellData event handler!

The price to be paid for this approach is a small loss of speed
because the style cell has to be searched within the list of all
formatting styles in order to get the style index. But if you don't
have too many of the style cells I'd guess that the difference won't
be noticed at all.

----------------------------------------------------------------------
rvk · 2014-07-14 15:52

What if i wanted the last row (summary or something) in a different
color (say light-yellow) which is not used in the above cells?
Can i use FindCell for rows that aren't yet filled in? (because i need
to do this before the NeedCellData)

And what if i want a light-red background for all negative numbers?
I don't know yet what cell-coordinate these are so i can't define a
template-cell beforehand.
(And i can hardly fill in the first row with these templates)

Isn't there a possibility to define a "virtualcell" (not used in the
sheet or in the cells above) with formatting which you can use during
filling the cells?

----------------------------------------------------------------------
Mike.Cornflake · 2014-07-14 16:03

Would the following work?  Create a second worksheet.  Set up your
template cells in that spreadsheet.  Then use NeedCellData to populate
the first worksheet?
And maybe delete the second worksheet after all the NeedCellData stuff
is finished?   Dunno, just an idea...

----------------------------------------------------------------------
wp · 2014-07-14 16:11

To which cells the template cells are applied is determined in the
OnNeedCellData handler. So, if you want the last row in yellow you say

Code: [Select]var
  yellowCell: PCell;

  // you can write the yellow style anywhere in the worksheet, say in
A2
  worksheet.WriteBackground(1, 0, scYellow);
  yellowCell := worksheet.FindCell(1, 0);
...

procedure TSomething.NeedCellDataHandler(Sender: TObject; ARow, ACol:
Cardinal;
  var AValue: variant; AStyle: PCell);
begin
  if ARow = workbook.VirtualRowCount-1 then
    AStyle := yellowCell;
  ...

Quotered background for all negative numbers?You define a cell with
red background and in OnNeedCellData you look at the number value
retrieved from the database (as an example); if it is negative you use
the cell with the red background as a style cell, otherwise you can
use another one, or nil for default formatting.

Quote possibility to define a "virtualcell" (not used in the sheet or
in the cells above) No. Because when the cell is written you have to
specify the style index, this is the index of the style in the list of
known formats. This list is built by the writer before beginning to
write the cells and stores it to file. As we have discussed before it
is not possible to modify this list later in case of BIFF format
(except you pay me for excessive debugging hours...)

QuoteSet up your template cells in that spreadsheet.  Then use
NeedCellData to populate the first worksheet?I think that this should
work because the writer collects the styles from all sheets of the
workbook. Just like in xlsx file format: there is a styles.xml which
is valid for all sheets.

----------------------------------------------------------------------
rvk · 2014-07-14 16:56

Quote from: Mike.Cornflake on July 14, 2014, 06:03:40 pmWould the
following work?  Create a second worksheet.  Set up your template
cells in that spreadsheet.  Then use NeedCellData to populate the
first worksheet?
And maybe delete the second worksheet after all the NeedCellData stuff
is finished?   Dunno, just an idea...
Haha... I just figured it out...
It's a bit counter-intuitive but if you use WriteFontStyle for cell
0,0 etc.... you create the template cells.
After that you're using NeedCellData to actually populate these cell's
and the data (and styles in it) will be deleted and replaced with the
actually data and link to the style-index.
(It wasn't clear to me you used the sheet at first for creating the
templates after which these will be destroyed and replaced.)

I'm not saying the building of the templates should be done during
population (in NeedCellData) but i still think the building of the
styles could be done virtually (without using the worksheet) but that
would mean rewriting the gathering of the styles (not from the sheet
but from memory) and i'm not sure if that would mess up the other
formats.

But the way you did it now works good too. (It's just some getting
used to using the first cells of the sheet when these will be thrown
away)

There could even be a small object written which does all this work
for you (without you needing to mess with the sheet yourself).
It could use the first row of the sheet and populating it with the
styles you want.
It could use template-names for easy access.

Code: [Select]MyTemplates := TTemplateCells.Create;
MyTemplates.WriteFontStyle('just_bold',  [fssBold]);
MyTemplates.WriteFontStyle('bold_and_gray',  [fssBold]);
MyTemplates.WriteBackground('bold_and_gray',  scGray);
MyTemplates.WriteFontStyle('negative',  [fssBold]);
MyTemplates.WriteBackground('negative',  scRed);
MyTemplates.PopulateTemplateCells; // <- this will populate an
internal PCell structure
and in NeedCellData
Code: [Select]...
AData := myValue
if ARow = 0 then AStyle := MyTemplates.GetTemplate('just_bold');
if myvalue < 0 then  AStyle := MyTemplates.GetTemplate('negative');
...
Internally in that object the WriteFontStyle would look up 'just_bold'
in it's own index (TStringList) to see which row/column and set the
style in the worksheet there.

Just a thought for someone (maybe me) to create later, because that
would stand apart from fpspreadsheet itself.
(It would just be a small helper-object)

----------------------------------------------------------------------
wp · 2014-07-14 18:52

Well - the template cells are not destroyed, they are regular cells of
the worksheet. They just lend their style to the virtual cells
written. Maybe it is clearer when I post some code. This is some
simplified code of the WriteWorksheet method of TsSpreadOOXMLWriter:

Code: [Select]procedure TsSpreadOOXMLWriter.WriteWorksheet(CurSheet:
TsWorksheet);
var
  r, c: Cardinal;  // row and column index
  value: Variant;  // value to be used in the virtual cell
  styleCell: PCell;  // PHYSICAL cell to provide the format, it is
part of the workbook, its position and value are ignored
  lCell: TCell;      // VIRTUAL cell being written - it does not
belong to the workbook
begin
  WriteXMLHeaderForWorksheet;
 
  if (woVirtualMode in Workbook.WritingOptions) and
Assigned(Workbook.OnNeedCellData)
  then begin
    for r := 0 to Workbook.VirtualRowCount-1 do begin
      for c := 0 to Workbook.VirtualColCount-1 do begin
        // Reset the virtual cell to default values
        FillChar(lCell, SizeOf(lCell), 0); 

        // Provide default values for return of event handler
        value := varNull;
        styleCell := nil;

        // call event handler; this overwrites the defaults of the
previous step
        Workbook.OnNeedCellData(Workbook, r, c, value, styleCell);

        // if the event handler returned a style cell the style cell
is copied over to the virtual cell
        if styleCell <> nil then
          lCell := styleCell^;

        // We set row and column index of the virtual cell such that
it appears in the file where we want it to be.
        lCell.Row := r;
        lCell.Col := c;

        // Then we analyze the value provided by the event handler
and, depending on its type,
        // we put the value into the correct content field of the
virtual cell.
        if VarIsNull(value) then
          lCell.ContentType := cctEmpty
        else
        if VarIsNumeric(value) then begin
          lCell.ContentType := cctNumber;
          lCell.NumberValue := value;
        end else
          // etc

        // Now we have everything we need to write the virtual cell to
the stream. Depending on the
        // ContentType of the cell WriteCallBack calls the writing
code for Number, Text etc. cells.
        // Note that this does not create cells in the worksheet.
        WriteCellCallback(@lCell, FSSheets[FCurSheetNum]);
      end;
    end;
  end else
    // Write regular ("physical") cells
    ...

----------------------------------------------------------------------
rvk · 2014-07-14 19:19

Quote from: wp on July 14, 2014, 08:52:26 pmWell - the template cells
are not destroyed, they are regular cells of the worksheet. They just
lend their style to the virtual cells written.Yeah... it was already
clear to me you used the original structure of the worksheet (the same
as you use without the woVirtualMode) for the styling cells. And the
wording "destroyed" was wrong of me... (I already dove into the
source) It's just... when using the woVirtualMode you shouldn't
consider the (cells in the) worksheet as actually (cells in the)
worksheet anymore but as a style container. So... that takes some
getting used to

----------------------------------------------------------------------
wp · 2014-07-14 21:48

VirtualMode now has found its way into the fpspreadsheet wiki.

----------------------------------------------------------------------
rvk · 2014-07-15 07:57

Quote from: wp on July 14, 2014, 11:48:27 pmVirtualMode now has found
its way into the fpspreadsheet wiki.
I take it woVirtualMode hasn't found it's way in fpsopendocument.pas
yet...
(But you've already been extremely busy the last few days )

I did notice a blowup in time using woSaveMemory with the sfExcel8
format.
I haven't gotten profiler to work yet so i don't know where the reason
lies but will investigate.
Or it is because writing small-pieces every time is very slow.
In that case sfExcel8 doesn't lend itself so good for using
woSaveMemory because without it, it is lightning fast.

(Maybe we still need a TFileStream with a buffering mechanisme)

Code: [Select]Running: Building TsWorkbook and Writing 3 formats
  All strings with Windows 7 pro and SSD drive
----------                                       .xlsx   .ods   .xls
Rows x Cols WritingOptions                Build  Write  Write  Write
Rows x Cols WritingOptions                Build  Write  Write  Write
10.000x100  [                           ]   0,7    5,3    2,9    1,1
10.000x100  [woVirtualMode              ]   0,0    6,4    0,0    1,3
10.000x100  [               woSaveMemory]   0,7   12,3    8,1   30,1
10.000x100  [woVirtualMode, woSaveMemory]   0,0   13,7    0,0   31,0
---------
40.000x100  [                           ]   3,0   21,4   12,1    4,1
40.000x100  [woVirtualMode              ]   0,0   25,0    0,0    5,4
40.000x100  [               woSaveMemory]   3,0   48,7   30,6  125,5
40.000x100  [woVirtualMode, woSaveMemory]   0,0   53,4    0,0  128,3
----------
Ready

Keep up the great work. 


Edit: Yeah... in WriteLabel (sfExcel8) you do about 7 (+ whatever is
done in WriteXFIndex) separate writes to the tfilestream.
When building an internal small buffer in that procedure and writing
that to stream you could cut down the time from 30 seconds to 4 (for
10.000) and from 125 down to 20 (for 40.000 records). (and that's just
in WriteLabel. I didn't check the other procedures.) Besides buffering
in your procedure itself, a buffering tfilestream would solve all this
too. I haven't found one yet, though.

----------------------------------------------------------------------
wp · 2014-07-15 14:07

QuoteI take it woVirtualMode hasn't found it's way in
fpsopendocument.pas yetCorrect. Still scared of the empty cell
handling of ods...

Quotein WriteLabel (sfExcel8) you do about 7 separate writes to the
tfilestream.Exact. And all biff units are full of "Stream.WriteByte",
"WriteWord" etc... It would help a lot if all biff records were
written at once instead of writing their parts separately.

But I am not sure if this is really the main issue. Windows supports
buffered writing, i.e. every call to "Stream.WriteByte" first goes
into a buffer anyway without me doing anything.

My feeling is that due to the size limitations of biff (max 65536 rows
x 256 columns) there is less urgent need for the woSaveMemory option
here. Therefore I'll postpone this issue until I'm ready for a lot of
typing again...

Quotebuffering tfilestreamDon't put too much effort into this. Unless
included with fpc I am hesitant to add dependencies on other external
libraries to the fpspreadsheet packages. It always a pain for the user
being forced to install yet another package which may lead to
instabilities and issues.

----------------------------------------------------------------------
wp · 2014-07-15 15:03

Couldn't resist - and I've got to correct myself: there is a factor of
4 speed increase of the virtualmode demo for biff8 when complete
records of number and label cells are written instead of single bytes
and words... Is contained in rev 3321 (number cells for all biff,
label cells for biff8 only).

----------------------------------------------------------------------
rvk · 2014-07-15 15:04

Quote from: wp on July 15, 2014, 04:07:35 pmBut I am not sure if this
is really the main issue. Windows supports buffered writing, i.e.
every call to "Stream.WriteByte" first goes into a buffer anyway
without me doing anything.
I don't think Windows (or Linux) is buffering these writes.
I'm not sure why not, but i did some testing and when using an
internal buffer (if only in the WriteLabel-function) the time get
reduced immensely.
There are a lot of discussions about it on the net (in combination
with TFileStream).

I tried one myself, TaaWriteBufferFilter (which was hard to find),
with a small test-loop (10.000 times WriteBuffer(V,1)) which reduced
the writing of a file from 36,1 seconds to 0,1 second.

And if you disable the WriteBytes in xlsbuff8.pas in WriteLabel and
leave the last WriteBuffer standing (just for testing), you'll see
these are the reason for the slowdown.

QuoteMy feeling is that due to the size limitations of biff (max 65536
rows x 256 columns) there is less urgent need for the woSaveMemory
option here. Therefore I'll postpone this issue until I'm ready for a
lot of typing again...
Completely understandable (and no problem)...

QuoteDon't put too much effort into this. Unless included with fpc I
am hesitant to add dependencies on other external libraries to the
fpspreadsheet packages. It always a pain for the user being forced to
install yet another package which may lead to instabilities and
issues.
The TFileStreamBuffered (or TaaWriteBufferFilter) could, of course, be
a separate unit in fpsreadsheet itself (like fpszipper.pp. It would
only be a small one of about 180 lines max. and fpszipper.pp is way
bigger). No need for external dependencies. It could even be
transferred to fpc at one point. I was surprised there was no standard
buffering in TFileStream by default. (I'm still looking)

But you're correct... that's for later.
(I'll let you know if and when i have a working sample)

B.T.W. I noticed that fpsopendocument.pas implemented the writing
itself directly in .WriteToFile but for xlsxooxml.pas it is in
.WriteToStream which is called from .WriteToFile.
In fpsopendocument.pas the .WriteToStream is unsupported yet. I'm
assuming this is still on your (probably very big) todo-list

----------------------------------------------------------------------
rvk · 2014-07-15 15:17

Quote from: wp on July 15, 2014, 05:03:20 pmCouldn't resist - and I've
got to correct myself: there is a factor of 4 speed increase of the
virtualmode demo for biff8 when complete records of number and label
cells are written instead of single bytes and words... Is contained in
rev 3321 (number cells for all biff, label cells for biff8 only).
Numer and label cells would be more than sufficient for now (as they
are used the most).

Yeah, now .xls is in line with the others again. (And it is fastest
again )

Revision 3321:
Code: [Select]Running: Building TsWorkbook and Writing 3 formats
  All strings with Windows 7 pro and SSD drive
----------                                       .xlsx   .ods   .xls
Rows x Cols WritingOptions                Build  Write  Write  Write
40.000x100  [                           ]   3,0   21,6   12,1    3,9
40.000x100  [woVirtualMode              ]   0,0   25,4      -    5,3
40.000x100  [               woSaveMemory]   2,9   48,1   30,3   21,2
40.000x100  [woVirtualMode, woSaveMemory]   0,0   53,5      -   25,2
----------
Ready

----------------------------------------------------------------------
wp · 2014-07-15 15:28

Quotewhich reduced the writing of a file from 36,1 seconds to 0,1
second.That's impressive. Even xlsxooxml and ods would benefit...

BUT: when adding code of other people to fpspreadsheet we must
carefully study their licenses.

QuoteI noticed that fpsopendocument.pas implemented the writing itself
directly in .WriteToFile but for xlsxooxml.pas it is in .WriteToStream
which is called from .WriteToFile.
In fpsopendocument.pas the .WriteToStream is unsupported yet.
I know. Putting the writing stuff of ooxml into the WriteToFile method
probably would also solve the zipper filename issue that you reported?
But I am hesitant: According to my experience there is more
flexibility if the main writing goes into a stream. For example you
can create the stream as a filestream with shareDeny options currently
not used.

----------------------------------------------------------------------
BigChimp · 2014-07-15 15:50

Quote from: rvk on July 15, 2014, 05:04:27 pmThe TFileStreamBuffered
(or TaaWriteBufferFilter) could, of course, be a separate unit in
fpsreadsheet itself (like fpszipper.pp. It would only be a small one
of about 180 lines max. and fpszipper.pp is way bigger). No need for
external dependencies. It could even be transferred to fpc at one
point. I was surprised there was no standard buffering in TFileStream
by default. (I'm still looking)
I would strongly advise not to include such a unit in fpspreadsheet
but try to get it into FPC trunk instead. In my experience, FPC devs
are open to well-thought out code that offers functionality benefits.

We could indeed perform a similar action to the use of fpszipper:
- use our own copy of the buffered stream unit if FPC 2.6.4 or lower
is used so it still compiles
- pass off the maintenance burden of such a generic unit by
referencing the unit in FPC 2.7.1+ once it is included
This also paid off for fpszipper which started out as a copy of the
existing FPC zipper code with fixes that apparently were essential to
fpspreadsheet (before my time); these fixes were merged into FPC trunk
and eventually trunk got more features and fixes (e.g. zip64 support);
fpspreadsheet now gets these for free (if built with FPC trunk of
course).

----------------------------------------------------------------------
rvk · 2014-07-15 17:10

Quote from: wp on July 15, 2014, 05:28:47 pmI know. Putting the
writing stuff of ooxml into the WriteToFile method probably would also
solve the zipper filename issue that you reported? But I am hesitant:
According to my experience there is more flexibility if the main
writing goes into a stream. For example you can create the stream as a
filestream with shareDeny options currently not used.
I was actually suggesting it the other way around. Moving the stuff in
WriteToFile in fpsopendocument.pas to WriteToStream. That way the
xlsxooxml and fpsopendocument are more alike and you'll also have the
WriteToStream in fpsopendocument.pas (which is now an unsupported
function).

(as i see it this would be a simple moving of some code in
fpsopendocument.pas because the WriteToFile already works with
streams. The code could almost be identical to the one in
xlsxooxml.pas)

I would solve the zipper-filename issue with assigning a temporary
filename, so if there are bigger files >256kb the filename wouldn't be
".tmp" but "temp_something.tmp" which in my testing works better then
the empty filename. I will do some more testing on this to make sure
what the issue is exactly.

----------------------------------------------------------------------
wp · 2014-07-15 18:00

QuoteI was actually suggesting it the other way around. Moving the
stuff in WriteToFile in fpsopendocument.pas to WriteToStream.This,
actually, was what I was trying to say with being "hesitant".

----------------------------------------------------------------------
wp · 2014-07-18 23:01

The new unit fspStreams.pas in rev 3331 contains a buffered stream
which behaves like a memory stream but swaps to disk when a given
buffer size is exceeded. There are several successful test cases in
the tests suite which give me the feeling that the interplay between
both streams is correct.

Applied to fpspreadsheet there is the expected speed-up of virtual
mode when run with the woSaveMemory option. Using a 1MB buffer, the
writing of 60.000x100 cells to biff8 does not take any longer than
using a pure memory stream, and going down to a 1kB buffer requires
only 2 additional seconds to the 14 sec of the memory stream case. The
same for xlsx.

I'll do some more testing, and if there are no non-resolvable
regressions I'll abandon the saveMemory option and use the buffered
stream instead of memory streams.

----------------------------------------------------------------------
wp · 2014-07-19 13:33

The buffered stream is now (rev 3337) used for writing within the
entire fpspreadsheet package (except for the zipper which I did not
touch) if option woBufStream is set in the workbook's WritingOptions
(this replaces the old "woSaveMemory" option which is no longer named
appropriately because the buffered stream also saves time).

A particularly impressive speed improvement can be found for biff2
files for which 64000x100 cells write in 1.7 seconds now, while it
took 31 seconds using the old FileStream.

I am not sure to activate woBufStream by default because I've see
cases where writing was a bit slower than before (using a pure memory
stream in these cases).

----------------------------------------------------------------------
rvk · 2014-07-20 13:44

Very impressive

I see you've created you're own TBufStream (I knew that would make a
huge difference) 

I'm still struggling with some weird memory problem when creating and
destroying multiple spreadsheets. I've attach my fpsspeedtest.zip
project where i time the creation and writing of different test-
spreadsheets.

Here is the result:
Code: [Select]Running: Building TsWorkbook and Writing to different
formats
  All strings with Windows 7 pro and SSD drive
----------                        .ods  .xlsx  biff8  biff5  biff2
Rows x Cols W.Options     Build  Write  Write  Write  Write  Write
10.000x100  [          ]    0,7    2,8    4,8    0,9    0,5    1,8
10.000x100  [woVM      ]    0,0    0,0    5,7    1,2    0,7    2,6
10.000x100  [      woBS]    0,7    5,2    5,7    0,9    0,5    0,3
10.000x100  [woVM, woBS]    0,0    0,0    6,5    1,2    0,7    0,6
----------
20.000x100  [          ]    1,4    5,5    9,7    1,9    1,0    3,6
20.000x100  [woVM      ]    0,0    0,0   11,5    2,5    1,5    5,2
20.000x100  [      woBS]    1,4    9,5   11,2    1,9    1,0    0,6
20.000x100  [woVM, woBS]    0,0    0,0   13,2    2,5    1,5    1,3
----------
30.000x100  [          ]    2,2   xxxx   xxxx   xxxx   xxxx    5,5
30.000x100  [woVM      ]    0,0    0,0   xxxx   xxxx   xxxx    8,0
30.000x100  [      woBS]    2,3   14,3   xxxx   xxxx   xxxx   xxxx
30.000x100  [woVM, woBS]    0,0    0,0   xxxx   xxxx   xxxx   xxxx
----------
40.000x100  [woVM      ]    0,0    0,0   xxxx   xxxx   xxxx   10,7
40.000x100  [woVM, woBS]    0,0    0,0   xxxx   xxxx   xxxx   xxxx
----------
Ready
(xxxx gives an OutOfMemory error)

Somehow after creating, writing and destroying TsWorkbook there seems
to be some memory-loss. If it's a problem with not freeing memory or
with some weird memory fragmentation, i don't know. When my test-
program comes to 30.000x100 it crashes with an out of memory. (using
except i can continue). When doing the test only for 40.000x100 it
works perfectly without problem (but looking at the taskmanager there
is still a lot of memory used compared to before pressing the button).

When doing the "Rows := 4 to 4" alone i get:
Code: [Select]Running: Building TsWorkbook and Writing to different
formats
  All strings with Windows 7 pro and SSD drive
----------                        .ods  .xlsx  biff8  biff5  biff2
Rows x Cols W.Options     Build  Write  Write  Write  Write  Write
40.000x100  [          ]    2,9   11,3   19,5    3,7    2,0    7,1
40.000x100  [woVM      ]    0,0    0,0   23,1    5,0    3,0   10,3
40.000x100  [      woBS]    2,9   18,3   21,7    4,3    2,0    1,2
40.000x100  [woVM, woBS]    0,0    0,0   25,3    5,6    3,0    2,6
----------
Ready

So, there is indeed a huge speed difference, but if there is some
memory-loss you can't keep your program running and doing multiple of
these actions. If there is some sort of memory fragmentation in FPC
then i wonder is there something i could program to resolve that.

When the program starts it uses about 10MB memory but when doing one
or more spreadsheet-actions and destroying the workbook it uses a lot
more. Is there a function i'm missing to return the missing memory to
"the pool"?

----------------------------------------------------------------------
wp · 2014-07-20 14:45

Nice program. Would you allow me to add it to the fpspreadsheet
package?

As for the memory leak: It really looks that there is one. Activate
the option "Use heaptrc unit" in the project options / Compiler
options / Debugging. Rebuild the package and the project (I used the
test_virtualmode which is simpler), and at the end you'll see a list
of lines where memory is occupied but never released.

That's a huge list here because of the many cells. The memory leak
occurs in the OnNeedCellData event handler where the string for the
cell contents is built. This is a rather harmless line, but I guess
the problem is one line later where the string is assigned to a
variant.

To be honest I don't have much experience with variants in Lazarus -
this is the second time that I am using them, and the second time that
I am having problems. I am just applying my Delphi knowledge, but it
looks as if variants in Lazarus behave differently from Delphi. Could
this be? In particular, how can I release the memory occupied by a
string converted to a variant?

Probably it would be better to avoid the variant at all and use a
TCell as return value of the event handler. Then value and style could
be packed into the same variable.

Let me think about it for some time...

----------------------------------------------------------------------
rvk · 2014-07-20 16:10

Quote from: wp on July 20, 2014, 04:45:38 pmNice program. Would you
allow me to add it to the fpspreadsheet package?
Yes, you can use it in the fpspreadsheet package. Also feel free to
add any improvements when you have them.

QuoteAs for the memory leak: It really looks that there is one.
Activate the option "Use heaptrc unit" in the project options /
Compiler options / Debugging.I didn't know about the "Use heaptrc
unit" yet (just beginning with Lazarus  )

QuoteThat's a huge list here because of the many cells. The memory
leak occurs in the OnNeedCellData event handler where the string for
the cell contents is built. This is a rather harmless line, but I
guess the problem is one line later where the string is assigned to a
variant.
It's not just in the OnNeedCellData where there is memory-loss.
When doing a simple console-app with just creating an empty workbook
and writing it (.ods the old way with empty [] WritingOptions) there
is some loss. (other formats have no problem, only sfOpendocument has
some memory-loss)
With heaptrc on, try this in an console app:
Code: [Select]program project1;
{$mode objfpc}
uses
  fpspreadsheet, laz_fpspreadsheet;
var
  MyWorkbook: TsWorkbook;
  MyWorksheet: TsWorksheet;
begin
  writeln('press enter to start');
  readln;
  MyWorkbook := TsWorkbook.Create;
  try
    MyWorksheet := MyWorkbook.AddWorksheet('Sheet1');
    MyWorkbook.WritingOptions := [];
    MyWorkbook.WriteToFile('c:\temp\test.ods', sfOpenDocument, true);
  finally
    MyWorkbook.Free;
    writeln('ready');
    readln;
  end;
end.
You'll see a loss of memory: (3 unfreed memory blocks : 64)
Code: [Select]Heap dump by heaptrc unit
307 memory blocks allocated : 15178850/15179768
304 memory blocks freed     : 15178786/15179696
3 unfreed memory blocks : 64
True heap size : 753664 (128 used in System startup)
True free heap : 753232
Should be : 753272
Call trace for block $0028D628 size 16
  $0040C378
  $00417999  TSCUSTOMNUMFORMATLIST__ADDFORMAT,  line 4924 of
fpspreadsheet.pas
  $00451F3D  TSSPREADOPENDOCNUMFORMATLIST__ADDBUILTINFORMATS,  line
308 of fpsop
endocument.pas
  $00417703  TSCUSTOMNUMFORMATLIST__CREATE,  line 4865 of
fpspreadsheet.pas
  $00458A69  TSSPREADOPENDOCWRITER__CREATENUMFORMATLIST,  line 2233 of
fpsopendo
cument.pas
  $00418477  TSCUSTOMSPREADWRITER__CREATE,  line 5338 of
fpspreadsheet.pas
  $0045CB96  TSSPREADOPENDOCWRITER__CREATE,  line 3083 of
fpsopendocument.pas
  $00416C2E  TSWORKBOOK__CREATESPREADWRITER,  line 4130 of
fpspreadsheet.pas
Call trace for block $0028D5C8 size 20
  $0041783B  TSCUSTOMNUMFORMATLIST__ADDFORMAT,  line 4889 of
fpspreadsheet.pas
  $00417999  TSCUSTOMNUMFORMATLIST__ADDFORMAT,  line 4924 of
fpspreadsheet.pas
  $00451F3D  TSSPREADOPENDOCNUMFORMATLIST__ADDBUILTINFORMATS,  line
308 of fpsop
endocument.pas
  $00417703  TSCUSTOMNUMFORMATLIST__CREATE,  line 4865 of
fpspreadsheet.pas
  $00458A69  TSSPREADOPENDOCWRITER__CREATENUMFORMATLIST,  line 2233 of
fpsopendo
cument.pas
  $00418477  TSCUSTOMSPREADWRITER__CREATE,  line 5338 of
fpspreadsheet.pas
  $0045CB96  TSSPREADOPENDOCWRITER__CREATE,  line 3083 of
fpsopendocument.pas
  $00416C2E  TSWORKBOOK__CREATESPREADWRITER,  line 4130 of
fpspreadsheet.pas
Call trace for block $00294558 size 28
  $00458A69  TSSPREADOPENDOCWRITER__CREATENUMFORMATLIST,  line 2233 of
fpsopendo
cument.pas
  $00418477  TSCUSTOMSPREADWRITER__CREATE,  line 5338 of
fpspreadsheet.pas
  $0045CB96  TSSPREADOPENDOCWRITER__CREATE,  line 3083 of
fpsopendocument.pas
  $00416C2E  TSWORKBOOK__CREATESPREADWRITER,  line 4130 of
fpspreadsheet.pas
  $00416DAD  TSWORKBOOK__WRITETOFILE,  line 4295 of fpspreadsheet.pas
  $00401916  main,  line 15 of fpsspeedtest_console2.lpr
  $0040FE01
  $00750053

Quote... but it looks as if variants in Lazarus behave differently
from Delphi. Could this be?
In particular, how can I release the memory occupied by a string
converted to a variant?
I always thought a variant would clean itself up (reference counted)
but i found this topic (in combination with params of a dataset) where
it is indicated there might be some problems with variants. I haven't
searched bugtracker yet. Also doing a "varClear(Value)" doesn't seem
to resolve the problem (if i done it correctly) so if there is really
a problem with variants in FPC we should try to find out what that is.
(Variants should be working fine in this case, even in FPC)

(I saw TCell didn't use a variant for value while it could have done
so to save memory)

----------------------------------------------------------------------
wp · 2014-07-20 16:28

Thanks for doing my debugging work

This was an easy one: the TsSpreadOpenDocuWriter.Destroy was missing
the call of the inherited destructor --> fixed in rev. 3351

QuoteI saw TCell didn't use a variant for value while it could have
done so to save memorySeems that it was a wise decision of the initial
makers of fpspreadsheet...
A few bytes can also be saved by using a variant record - I'll try
that

----------------------------------------------------------------------
rvk · 2014-07-20 16:58

Regarding the memory-leak in OnNeedCellData (variant?) if i comment
out line 920 in xlsxooxml.pas (rev.3351) the memory-leak goes away.
(That's the fillchar line of lCell)

Code: [Select]919 for c := 0 to Workbook.VirtualColCount-1 do begin
920   //FillChar(lCell, SizeOf(lCell), 0);
921   CellPosText := CurSheet.CellPosToText(r, c);

(But i'm completely baffled as to why this is)

Edit: I think this has to do with the assignment of (or use of) a
variant to for example lCell.UTF8StringValue. After doing so you'll
need to clean up the variant before doing a FillChar on that lCell.

I think it throws up the reference count of the variant somehow (so
automatic cleanup can't or won't be done)

(I'm not sure how to clean that up properly, though)

----------------------------------------------------------------------
rvk · 2014-07-20 17:35

I've found it 

Look at this small samplecode:
Code: [Select]var
  V: Variant;
  R:record
      S: ansiString;
    end;
begin
  V:='Hellow World';
  R.S:=V;
  WriteLn(R.S);
  //R.S:=#0;     // <-- without this you get a memory error
  fillchar(R, SizeOf(R), #0);
  WriteLn(R.S);
end.
When doing a fillchar on a record containing a string you first need
to destroy the string or else you get a memory leak.

I did the following to xlsxooxml.pas and it worked:
Code: [Select]      for c := 0 to Workbook.VirtualColCount-1 do begin
        lCell.UTF8StringValue := #0;
        FillChar(lCell, SizeOf(lCell), 0);
        CellPosText := CurSheet.CellPosToText(r, c);

In this case i only did it for UTF8StringValue but it needs to be done
for all string-types in lCell before fillchar'ing lCell. Of course in
this loop it is only UTF8StringValue but i don't now about other
places.

BTW I'm don't think this has anything to do with variants, it's just
that the string assigned is never freed (because you do a fillchar).

----------------------------------------------------------------------
Rails · 2014-07-20 17:52

I think that happens because the fillchar operation copies the
original string leaving the original string in place when the program
is closed. As a result, memory error.

I ran into a similar issue with a dynamic array of records.

----------------------------------------------------------------------
rvk · 2014-07-20 18:00

Quote from: Rails on July 20, 2014, 07:52:33 pmI think that happens
because the fillchar operation copies the original string leaving the
original string in place when the program is closed. As a result,
memory error.Well ansistring (in the lCell) is just a pointer to a
string in memory (otherwise the records itself would have been huge).

When overwriting the record (and thus the stringpointer) with #0's you
only reset the record (overwriting the pointer) but the original
string is still in memory (not referenced anymore). So you may never
do a fillchar on a record containing a non-empty string. You always
have to empty/free the string first.

----------------------------------------------------------------------
wp · 2014-07-20 18:20

Fixed in rev 3352. Thanks!

----------------------------------------------------------------------
marcov · 2014-07-20 18:32

Quote from: rvk on July 20, 2014, 08:00:23 pm When overwriting the
record (and thus the stringpointer) with #0's you only reset the
record (overwriting the pointer) but the original string is still in
memory

To be exact: it's reference-count isn't decremented. Since only the
final decrement releases memory, it is a memory leak.

Simply assigning '' to the string is enough.  (that decreases the
current ref count and assigns FPC_EMPTY_STRING (or something like it)
which is a constant in the FPC rtl with double zero.)

----------------------------------------------------------------------
wp · 2014-07-22 16:13

The current trunk version of fpspreadsheet contains the speed test
application posted here by rvk, thanks a lot (folder
examples/fpsSpeedTest). I modified it to allow changing of parameters
(number of rows, format seletion etc. - they are stored in an ini
file) and added also a routine for reading speed (not fully complete).

I extended the BufStream to be usable for the spreadsheet readers as
well (all xls, ods, but not xlsx which does not have a reader at all
so far). A factor 3 speed improvement for reading of biff2 compared to
a few revisions ago. But I am not quite satisfied with this
improvement because biff2 still reads considerably slower than biff5
and biff8 - in fact, if I copy the biff2 FileStream to a memorystream
I get the same speed as for the other biffs, so something must still
be wrong with the BufStream...

VirtualMode is not yet implemented for reading, but will follow.

Due to these modifications the workbook's "WritingOptions" are no
longer named appropriately, and I renamed this property to "Options",
and the set elements have the prefix "bo" ("book options") now
(instead of "wo")

----------------------------------------------------------------------
wp · 2014-07-24 16:05

Sorry to rename identifiers again and again, but this is work in
progress...

Here's another one: The event OnNeedCellData is now called
OnWriteCellData. The reason is that there is now also a OnReadCellData
for virtual reading mode. The background is the same as for virtual
writing mode: save memory for reading large files by passing cells
through to the evaluating procedure without storing them in the
worksheet. There is a demo_virtualmode_read in the examples/other
folder, it requires the file created by demo_virtualmode_write (the
former test_virtualmode).

As expected task manager does not show a significant increase of
memory usage if a file is read in virtual mode.

----------------------------------------------------------------------
taazz · 2014-07-24 16:36

Quote from: wp on July 24, 2014, 06:05:02 pmSorry to rename
identifiers again and again, but this is work in progress...

Here's another one: The event OnNeedCellData is now called
OnWriteCellData. The reason is that there is now also a OnReadCellData
for virtual reading mode. The background is the same as for virtual
writing mode: save memory for reading large files by passing cells
through to the evaluating procedure without storing them in the
worksheet. There is a demo_virtualmode_read in the examples/other
folder, it requires the file created by demo_virtualmode_write (the
former test_virtualmode).

As expected task manager does not show a significant increase of
memory usage if a file is read in virtual mode.

So the only difference is that one is called from the reading methods
and the other from the writing methods? Doesn't this makes it harder
for the end users to use needing to write 2 different events for the
same thing ee supply the data?

Disclaimer : I Haven't used the components my self not even installed
them yet,  just a thought.

----------------------------------------------------------------------
wp · 2014-07-24 16:53

QuoteSo the only difference is that one is called from the reading
methods and the other from the writing methods? Doesn't this makes it
harder for the end users to use needing to write 2 different events
for the same thing ee supply the data?No, it is not possible to use
the same event for both actions because of different calling
parameters:

Code: [Select]  TsWorkbookWriteCellDataEvent = procedure(Sender:
TObject; ARow, ACol: Cardinal;
    var AValue: variant; var AStyleCell: PCell) of object;

  TsWorkbookReadCellDataEvent = procedure(Sender: TObject; ARow, ACol:
Cardinal;
    const ADataCell: PCell) of object;

----------------------------------------------------------------------
taazz · 2014-07-24 17:08

and where is the cell data handled on the read? if it has not data to
handle then why call it dataevent?

----------------------------------------------------------------------
wp · 2014-07-24 17:35

The data are contained within the cell.

The reason why there is a separate data value variable in the writing
event is that the OnWriteCellData can also handle formatting which is
taken from the StyleCell. This is a pointer to an existing template
cell in the worksheet. If the data element of this cell would be used
to pass also the data to the writer I see the risk that users tend to
invent their own formatting which has not been listed by the writer
before. There is a discussion of this topic futher up in this thread.

----------------------------------------------------------------------
taazz · 2014-07-24 17:44

well I'm not familiar with the design of the suit so I'll just say
this last thing and stop bugging you. The design seems a bit of an
onion one layer on top of the other needs to be redesigned I guess any
way thank you for you time, If I get any free time I'll take a closer
look on the suit after all I might needed for exporting and importing
my self too.