bugGNU Octave - Bugs: bug #59277, [octave forge] (io) xls2oct is...

 
 

bug #59277: [octave forge] (io) xls2oct is slow when a spreadsheet contains many text cells

Submitter:  Dennis <dizze>
Submitted:  Thu 15 Oct 2020 04:46:26 PM UTC
   
 
Category:  Octave Package Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Performance
Status:  Fixed Assigned to:  philipnienhuis
Originator Name:  Open/Closed:  * Closed
Release:  * other Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 26 Oct 2020 03:21:52 PM UTC, comment #30: 

[OT]
Actually this bug report is not to be hijacked for other issues:-)

Octave-7.0.0 (current development version; release due end of 2021?) has jsonencode & jsondecode.
The io package has a basic object2json but unmaintained (isn't my code).
Patch #9980 offers better JSON support. Please continue there, or in the help mailing list (there is a thread there on JSON).

Philip Nienhuis <philipnienhuis>
Group Member
Mon 26 Oct 2020 01:06:21 PM UTC, comment #29: 

Ok, thanks. When is Octave 6.1 due for release?

I thought there was no support for JSON, but I now see that there is an object2json function. Are there any plans to support import of JSONs? I use JSONlab for that now (works well in Octave with a few minor adjustments), but there are some annoying bugs. I think JSON lab is open source, so you could consider adopting that code?

Dennis <dizze>
Mon 26 Oct 2020 12:18:12 PM UTC, comment #28: 

Thank you for the checks :-)

Yes I hope to release io-2.6.3 very soon. The wait is for (temporary) JSON I/O I'd like to include (patch #9980). Hopefully that'll happen soon.

In fact I'd like to have io-2.6.3 in the mxe-octave installers for upcoming Octave-6.1 to avoid Windows users having to live with a slightly buggy io package version until Octave-7.1 happens.
We can always add some dedicated io package fixes in the installer, has happened before, but that's a second-best option IMO, and primarily for real bugs.

Philip Nienhuis <philipnienhuis>
Group Member
Mon 26 Oct 2020 11:38:28 AM UTC, comment #27: 


> If you want you can benchmark if you get still better performance than the version you made up in comment #24.


With the version I made in comment #24, my test sheet yields the following:

Elapsed time is 39.2871 seconds.
   #         Function Attr     Time (s)   Time (%)        Calls
---------------------------------------------------------------
  23           regexp            18.893      48.49           50
  32         cell2mat             8.556      21.96        20060
  13 __OCT_xlsx2oct__             2.719       6.98           10
  24          cellfun             1.719       4.41       100350
  26              cat             1.554       3.99        20070


With your final version posted in comment #26, it yields:

Elapsed time is 37.3439 seconds.
   #         Function Attr     Time (s)   Time (%)        Calls
---------------------------------------------------------------
  23           regexp            18.376      49.58           50
  32         cell2mat             8.250      22.26        20050
  13 __OCT_xlsx2oct__             2.645       7.14           10
  26              cat             1.509       4.07        20060
  46       str2double             1.490       4.02           20


So very similar performance (I must say, the duration varies a bit, the first run with you latest code took 46s).

Then my real life application (three runs each):
- code from comment #24: 10.5 sec, 10.6 sec, 10.4 sec.
- Code from comment #27: 11.4 sec, 9.5 sec, 10.2 sec

Conclusion: the final code seems to perform fine. Many thanks for your quick and helpful response.

Just for my understanding (I know the details of releasing package updates), will you release a version 2.6.3 of the io package?

Dennis <dizze>
Sun 25 Oct 2020 09:01:59 PM UTC, comment #26: 

As a last action for this bug report I overhauled the code that transforms the combined regexp output into the raw output array.

Next I ran it with my testscript that checks the local bug spreadsheet collection, it looks like:

xfl = {(dir ("*.xlsx")).name}
xsz = [(dir ("*.xlsx")).bytes]
for ii=1:numel (xfl)
  [~, sheets] = xlsfinfo (xfl{ii}, "oct");
  for jj=1:size (sheets, 1)
    printf ("File %s, sheet %s, size %d, time ", xfl{ii}, sheets{jj, 1}, xsz(ii));
    tic;
    [num, txt, raw, lims] = xlsread (xfl{ii}, sheets{jj, 1}, "", "oct");
    printf ("%f\n", toc);
  endfor
endfor


For smaller spreadsheets the speed gains seem negligible but for larger ones, esp. with many strings, speed gains are impressive.

Attached is the __OCT_xlsx2oct__.m file I finally pushed here (previous improvements have been pushed earlier on):
http://hg.code.sf.net/p/octave/io/rev/3adc77a99b7b
If you want you can benchmark if you get still better performance than the version you made up in comment #24.

I'm curious, you can post your findings even while I'm going to close this bug report anyway; like you I think that most of the potential performance gains have been implemented.

Many thanks for your cooperation and suggestions.


(file #50099)

Philip Nienhuis <philipnienhuis>
Group Member
Sun 25 Oct 2020 07:32:48 PM UTC, comment #25: 


> So, replacing lines 204 and 205 as follows may speed up things further:
> idx.num = vi.row;
> idx.alph = vi.alph;

Good catch!

A little further down,

  • that 2nd assignment idx.alph = vi.alph seems a bit useless, vi.alph isn't referenced afterwards;

and

  • vi.col turns out to be identical to idx.alph, so even idx.alph isn't referenced anymore.


This is original code that AFAICR I haven't ever touched except for column and row limits. I need to look better to not screw up things, the program flow isn't exactly straightforward.
But some more optimization is due, absolutely.

Philip Nienhuis <philipnienhuis>
Group Member
Sun 25 Oct 2020 12:09:15 PM UTC, comment #24: 

An addition to my tests in comment #23:
I don't understand line 204 and 205. They are identical to L.197 and L.200 respectively, since L.202 reads idx.all = val(:, 1);

So, replacing lines 204 and 205 as follows may speed up things further:
idx.num = vi.row;
idx.alph = vi.alph;

Original:

㲫Elapsed time is 61.076 seconds.
   #         Function Attr     Time (s)   Time (%)        Calls
---------------------------------------------------------------
  23           regexp            34.614      57.06           70
  32         cell2mat            10.380      17.11        20080
  13 __OCT_xlsx2oct__             3.510       5.79           10
  46       str2double             2.828       4.66           30
  26              cat             2.434       4.01        20090


Replaced L.204 and L.205:

㲫Elapsed time is 40.6197 seconds.
   #         Function Attr     Time (s)   Time (%)        Calls
---------------------------------------------------------------
  23           regexp            19.325      48.00           50
  32         cell2mat             8.988      22.32        20060
  13 __OCT_xlsx2oct__             2.847       7.07           10
  24          cellfun             1.795       4.46       100350
  26              cat             1.629       4.05        20070


That seems to be useful some code cleaning :-)

Dennis <dizze>
Sun 25 Oct 2020 11:53:58 AM UTC, comment #23: 

Philip, regarding comment #20:

> Would you please look
> 1. if it still works
> 2. if you still have the expected speed gain
> ?


I tested you file and it still works. Also, it still has the performance improvement that I had before.

In addition, I created another test Excel sheet and small script (attached). With the _OCT_xlsx2oct_.m from v2.6.2 the output is:

Elapsed time is 161.678 seconds.
   #         Function Attr     Time (s)   Time (%)        Calls
---------------------------------------------------------------
  32         cell2mat            83.546      52.37       200060
  23           regexp            34.835      21.84       100060
  13 __OCT_xlsx2oct__            18.686      11.71           10
  24          cellfun             7.233       4.53       910340
  44       str2double             2.667       1.67           30


By far most of the time is spend on cell2mat

With the update you posted in comment #20, the output is:

Elapsed time is 53.296 seconds.
   #         Function Attr     Time (s)   Time (%)        Calls
---------------------------------------------------------------
  23           regexp            30.263      57.15           70
  33         cell2mat             8.937      16.88        20080
  13 __OCT_xlsx2oct__             3.050       5.76           10
  47       str2double             2.482       4.69           30
  26              cat             2.148       4.06        20090


That is a major improvement. I think we can consider this sufficiently fixed, as most time indeed is spend on regexp.

I had another look, which regexp is responsible, and it is NOT L.99. I changed the test script to loop only once and measured the time spend on all regexps I could find. Here are the results:

L.99:  Elapsed time is 0.353959 seconds.
L.142: Elapsed time is 0.201676 seconds.
L.145: Elapsed time is 0.665056 seconds.
L.175: N/A
L.197: Elapsed time is 0.694173 seconds.
L.200: Elapsed time is 0.726311 seconds.
L.204: Elapsed time is 0.601505 seconds.
L.205: Elapsed time is 0.548254 seconds.


Clearly L.99 is not the biggest time consumer. Next, I profiles L.197-200 and L.204-205. In both cases, the profiler shows that almost one second is spent on regexp. So in these line, regexp is indeed the function responsible for time consumption, not any other function.

In conclusion, if you want to optimize the speed any further, this seems to be the lines of code where most effort should go.

Regarding you question of credits, that is fine, thanks :-).

Cheers,
Dennis

(file #50093, file #50094)

Dennis <dizze>
Sat 24 Oct 2020 06:44:54 PM UTC, comment #22: 

Running through the collected bug report spreadsheets I found another reason for the for loop you stripped away, apart from comment #17: sharedStrings.xml can have nodes with (empty) contents like
<si><t/></si>
(while full nodes may look like <si><t>Some text</t></si> )
which gives differently sized empty cells in array "strings" (L.142). These are skipped by the regexp on L.145 and as a consequence nasty indexing errors are provoked further down.
I've found a simple fix, hopefully it doesn't eliminate the performance gains :-)

Along the way also found & fixed another, unrelated bug. Always handy to have such a collection :-)

BTW may I credit you in the io package NEWS file? I found your email address (I have some priviliges here) and I think I can guess your name.

Philip Nienhuis <philipnienhuis>
Group Member
Sat 24 Oct 2020 02:32:13 PM UTC, comment #21: 

A progress report:

First, I've adapted the title to the actual issue you entered this bug report for.

Then, again working with the code I remembered the way __OCT_xlsx2oct__.m is supposed to work (it's quite some time ago for me).

Most of the time is probably spent in the regexp call in L.99; that one actually parses all individual cells and extracts cell addresses, content types and values. For sharedString cells those "values" are actually pointers into the sharedStrings XML (0-based).
In 2017 Markus and I have spent quite some time figuring out how to make that regexp fast. I think there's not much to gain there anymore.

The code in section 2.B, ~L.139-151 where you worked on, merely parses the sharedStrings XML. As the name implies it holds text cell contents from the entire spreadsheet. That means that if the spreadsheet contains many different text cells, considerable time will be spent there. For most spreadsheets out there in the wild that's no big deal as they contain much more numerical than text data. But you are apparently in the niche where it does count.
You mentioned in comment #4 that presence of extra worksheets increases the time spent here; that fits perfectly with the sharedStrings XML containing the strings of the entire spreadsheet rather than just one worksheet.
Nothing can be done about it; it's just the way OOXML spreadsheets stick together.
Looking a bit further and after some experimenting I think there's no way the remaining for loop in L.150-155 can be vectorized.
All in all I think the speed increase you obtained (good work BTW) is about all we can get for the moment.

Finally, the individual sharedStrings values parsed in section 2.B are assigned to the "val" array where they replace the pointers mentioned above. That is merely assignment and is usually very fast.

There are additional steps that take time, e.g. figuring out column and row numbers from cell addresses and copying "val" contents into the proper locations in the output cellstr array. All those calls have also been vectorized as far as possible.

I started thinking about caching the parsed sharedStrings data, but soon I remembered I thought about that before :-) An obstacle is that if even only one text cell anywhere in the spreadsheet is changed the entire sharedStrings data is invalidated and needs to be reparsed/rebuilt. I suppose there are ways to do that and/or circumvent or mitigate it but:

  1. It'll add quite a bit of complexity, and the OCT interface already is a beast;
  2. AFAICS it'll only benefit a few use cases;
  3. I simply have other priorities,

so I'm not going to spend much more time on it now.

I'm always available for advice if you want to try that route.

Philip Nienhuis <philipnienhuis>
Group Member
Fri 23 Oct 2020 08:03:32 PM UTC, comment #20: 

OK, a first step from me.

Attached is an __OCT_xlsx2oct__.m with all your fixes and a somewhat amended suggestion cf. comment #19.

Would you please look

  1. if it still works
  2. if you still have the expected speed gain

?

I have a spreadsheet with ~162 strings, some deliberately with mixed formatting, where execution for reading it 100 times went down from 18.some to 16.some seconds, not much, but significant (it has many more numerical data), I tested along the lines of

xls = xlsopen ("<filename>", 0, "oct")
opts = struct ("formulas_as_text", 0)
>> tic; for ii=1:100; raw = __OCT_xlsx2oct__ (xls, 3, "", opts); end; toc
Elapsed time is 16.8628 seconds.
>> xls = xlsclose (xls)
xls = [](0x0)
>>


with patched __OCT_xlsx2oct__.m in the current directory
(check with "which __OCT_xlsx2oct__.m ")

In the mean time I'll check with my collection of bug report spreadsheets if I can spot any regressions.

(file #50077)

Philip Nienhuis <philipnienhuis>
Group Member
Wed 21 Oct 2020 11:47:49 AM UTC, comment #19: 

As an addition, a way to limit the for loop in lines 150-155, is to implement it as follows:

for n = find(cell2mat(cellfun(@numel,ctext,'uni',false))>1)
  ...
endfor


This may speed up things even a bit further, as it will only loop through those nested cells that have multiple indices.

Looking forward to hear if you think it is feasible to implement this change.

Dennis <dizze>
Tue 20 Oct 2020 12:31:22 PM UTC, comment #18: 

Ah I think you hit it on the nose - formatted strings, yes now I remember. The "complex strings" issue I mentioned a few comments below.
Good on you!

Unfortunately I can only look better at it tomorrow at the very earliest.

Philip Nienhuis <philipnienhuis>
Group Member
Tue 20 Oct 2020 11:48:55 AM UTC, comment #17: 

Philip,

I figured out why there is a for loop. The reason is that in some cases, the SharedStrings data contains multiple subentries per string. This happens when e.g. one word has been formatted. If the complete array called 'strings' (_OCT_xlsx2oct_.m line 142) is run through regexp, the number of cells at sublevel is not identical.

Suppose an string in Excel is "this red word". This is saved in SharedStrings as:

-<si>
        <t>this red word</t>
</si>


In this case regexp (string, '<t[^>]*>(.*?)</t>', "tokens") yields:

{
  [1,1] =
  {
    [1,1] = this red word
  }

}


But if the user colors the word "red" in red, the SharedStrings entry would look like something this:


<si>
        <r>
                <t xml:space="preserve">this </t>
        </r>
        <r>
                <rPr>
                        <color rgb="FFFF0000"/>
                </rPr>
                <t>red</t>
        </r>
        <r>
                <t> word</t>
        </r>
</si>


regexp would yield:


൓{
  [1,1] =
  {
    [1,1] = this
  }

  [1,2] =
  {
    [1,1] = red
  }

  [1,3] =
  {
    [1,1] =  word
  }

}


So, I guess the for loop is used to ensure that the lower level cell structures are being processed index by index, so that the final result is correctly mapped onto the correct index.

So I created a solution that accounts for this. It still includes a for loop, but that only executes cell2mat(cell2mat()) in case ctext{n} contains a multi index cell array. It flattens this. Afterwards, the complete cell array ctext is run through cell2mat(cell2mat()) in one go.

In my real life excel this works properly AND is much faster than the original code, because only some Excel strings contain formatting for single words. Please find attached the proposed code (lines 149-156).

(file #50026)

Dennis <dizze>
Tue 20 Oct 2020 07:35:21 AM UTC, comment #16: 

(Dennis, this comment just in between the scenes, I'm heavily occupied by other things)
I looked a bit in your __OCT_xlsx2oct__.m changes.
At first sight it boils down to just eliminating the for loop; that can explain a LOT of performance gain - for loops are known to grind execution speed down excessively.
(Hmmm I wonder how/why that for loop came there (or stayed there) in the first place.)

What you can also do is (as I suggested earlier) replace
cellfun (@function, ...)
by
cellfun ("function", ...)
(e.g., L.152 in your the __OCT_xlsx2oct__.m you uploaded lastly, in comment #14, and see if it has any significant effect.
(see https://savannah.gnu.org/bugs/?58445, look in comment #11 there)
I need to replace all cellfun invocations that way but lack time ATM.

(OK now I really need to get (paid) work done :-) )

Philip Nienhuis <philipnienhuis>
Group Member
Mon 19 Oct 2020 08:28:31 PM UTC, comment #15: 

I tested the supposed fix a bit further with the real dataset that I use in one of my projects. We can ignore the section in which I propose to apply regexp to the complete string cell array. It doesn't work.

So back to the solution I previously proposed. That too works with my formula test sheet (Excel2019 stringTest2.xlsx).

I can't think of a situation where the output of the regexp would be a cell array with multiple indices. The proposed solution simply takes the first index, which so far seems to work in every test sheet I have tried. As far as I can tell, the bug report bug #51512 doesn't seem to provide any reason to think this may not work. Philip, do you have any examples which would yield an output of the regexp with multiple cell array indices?

Dennis <dizze>
Mon 19 Oct 2020 07:44:15 PM UTC, comment #14: 

Philip,

I have read the bug report and comments of bug #51512 and looked for useable Excel files in that thread. Unfortunately I can't find any. Doesn't matter, I can make my own :-).

First test: an excel sheet with 1000x10 unique strings (see attachment 1 for excel (Excel2019 stringTest1.xlsx) and attachment 2 for _OCT_xls2oct_.m). Original code (executing line 146) yields:

Elapsed time is 20.6734 seconds.
   #         Function Attr     Time (s)   Time (%)        Calls
---------------------------------------------------------------
  32         cell2mat             9.210      45.08        20006
  23           regexp             6.132      30.02        10006
  13 __OCT_xlsx2oct__             2.054      10.06            1
  24          cellfun             0.864       4.23        90034
  40       str2double             0.505       2.47            3


Second try, commenting line 146, uncommenting line 147:

Elapsed time is 8.37976 seconds.
   #         Function Attr     Time (s)   Time (%)        Calls
---------------------------------------------------------------
  23           regexp             5.304      63.57        10006
  13 __OCT_xlsx2oct__             1.647      19.73            1
  32       str2double             0.439       5.26            3
  48           strrep             0.231       2.77            5
  26              cat             0.218       2.61            7


This is much faster, as previously concluded.

Try 3: commenting lines 145-148, uncommenting new line 149. This applies regexp to the complete cell array strings at once. This yields:

Elapsed time is 4.11125 seconds.
   #   Function Attr     Time (s)   Time (%)        Calls
---------------------------------------------------------
  93     regexp             3.161      69.93           30
 114 str2double             0.259       5.73            6
 112        cat             0.183       4.04           14
  49     strrep             0.178       3.95          100
 103     system             0.122       2.69            1


This is even faster. Since this implementation uses nested cell2mat as in the original code, it seems to me that this should be acceptable. The only difference is that it cuts out the for loop.

In order to put this to the test a bit more, I applied the code with a LibreOffice6 file with identical contents (attachment 3 (LibreOffice6 stringTest1.xlsx)). The performance is similar (perhaps even a bit faster). The spreadsheet content is correctly reflected in rawarr.

Finally, I created an Excel file with a number of formulas that results in strings. Some point to each other, or even with multilevel references. Others use the indirect function, and yet others concat two strings. See attachment 4 (Excel2019 stringTest2.xlsx). Using the fastest option (regexp applied to the complete string cellarray), functions correctly and the results are properly shown in rawarr.

I tend to conclude that this is a proper performance fix.


(file #50017, file #50018, file #50019, file #50020)

Dennis <dizze>
Sun 18 Oct 2020 07:28:20 PM UTC, comment #13: 

Thanks for testing xlsopen.m; so one issue got fixed.
In hindsight it was a bit of a dumb error by me, but I also remembered how and why I came there.

I dug up the bug report about formulas in sharedStrings.xml,
https://savannah.gnu.org/bugs/?51512
There's a fair amount of profiling involved there, maybe it can help you. The issue of shared formulas was touched there (and postponed, with good reason).
IIRC that bug report was also about, or a prelude to, other report(s) about complex strings. Maybe later on I'll stumble upon it/those.

As to your code:
The aim of cell2mat() is to get rid of extra cell "wrapping". Applying cellfun to cell2mat output looks like it is re-wrapped again.
No need to enclose the regexp call in parens, it is already enclosed in commas.

Maybe experimenting with vertcat or horzcat can help with unwrapping regexp's output as well, but I am not so familiar with those functions.

I'll set bug status provisionally back to "confirmed" until I have more time to look into it and/or help you recoding string reading in OCT interface.

Philip Nienhuis <philipnienhuis>
Group Member
Sun 18 Oct 2020 05:52:45 PM UTC, comment #12: 

I tested this line of code:
ctext{n, 1} = cellfun (@cell2mat, (regexp (strings{1, n}, '<t[^>]*>(.*?)</t>', "tokens")), 'UniformOutput', false);

It produces errors. I do not know how to use cellfun(@cell2mat,. ..) in this context such that it produces the correct output.

Dennis <dizze>
Sun 18 Oct 2020 05:22:37 PM UTC, comment #11: 


> Can you pleas try again with a patched xlsopen.m from here:


I tested it. First reinstated my java library and checked the functioning of the old xlsopen with my test script (previous comments). Indeed it did use UNO the first run, as previously. Next, I copied the new xlsopen, unloaded io and loaded io, and reran the test script. This time it uses OCT, as specified.

I next changed the script to specify using UNO, and that works as well: it now uses UNO. Since these are the only two interfaces I have available on my machine, I can't test xlsopen for the other interfaces, but based on my limited testing it seems to work as it should now.

Comming back to your question in comment #9:

> In your workflow, can you write spreadsheet contents to .csv files before processing?


I am afraid not. I am using Excel sheet to store some data, and created an Octave script that is actually being used as a API in a webservice. The excel file uses some simple formualas and formatting to properly check the validity of the data in it. I surely know that a dbase would be better, but that takes quite a bit implementation. Moreover, the Excel doesn't actually contain that much data. It is mostly strings and some numbers. The problem is that the commands used for the API cannot depend on previously loaded data, so every API call the excel is opened and loaded again. Consequently, there are quite a few loads, and the cell2mat used for SharedString really takes it's toll in performance. I will try and see if cellfun works better.

Dennis <dizze>
Sun 18 Oct 2020 03:57:07 PM UTC, comment #10: 

Can you pleas try again with a patched xlsopen.m from here:

http://hg.code.sf.net/p/octave/io/file/tip/inst/xlsopen.m

(I need to click "raw" in left column to init download)

just to check that you only get the interface you explicitly select?
Speed/performance will be tackled later.

Thanks

Philip Nienhuis <philipnienhuis>
Group Member
Sat 17 Oct 2020 11:09:58 AM UTC, comment #9: 

Dennis,

Yeah your initial bug report comprised a few issues, no worries, that often happens. I started to cherry pick the easiest one, interface issues, which as you described also gave rise to slow behavior and was reported elsewhere as well.
IMO proper functionality is always priority #1, speed comes later.

Then, in my comment #7:
"cell2mat (...)"
should read
"cellfun (...)"
(I shouldn't keep going on and on after midnight :-( )

"Diligence" - there's a reason that the current sharedStrings access code is what it is; and I must have various test spreadsheets lying around somewhere to check with. But again, presently I lack time to sort this all out, even to make it explicit, I simply have other priorities.
Later on I'm quite prepared (and curious) to pick it up.

As to speed / COM / databases / etc.:
In your workflow, can you write spreadsheet contents to .csv files before processing?
In the io package there's also csv2cell() that can read mixed .csv files (numeric and text) and does so blazingly fast. In fact it is what I use at work to read & process e.g., database exports of millions of lines (far beyond the limits of even .xlsx files). Once in a cell array data processing is usually very fast and can even be done in parallel using e.g., parcellfun.

Philip Nienhuis <philipnienhuis>
Group Member
Fri 16 Oct 2020 10:41:37 PM UTC, comment #8: 

You are right, there is some thread drift. My apologies for that. Do note that my initial motivation to dig into this was slow behavior. This was due to both reasons that you summarize.

Having said that, it is good to here that you have a fix for the first issue (using UNO interface when OCT is specified). I can't use the COM interface, because the code must run on a Unix system as well.

With regard to issue 2, I understand that proper diligence is required. Can we make it explicit what that means? What should be tested in order to be sure that the new code works properly in all cases?

In addition, I am certainly willing to check the cell2mat("<function name>", <cell array>, ...) option, but I need some directions. In your code cell2mat is used for a simple cell array, and no function name is entered. So how does this apply to this specific usage?

Dennis <dizze>
Fri 16 Oct 2020 08:40:32 PM UTC, comment #7: 

@Dennis,
your bug report title "behave unexpected" allows some thread drift and that is happening a bit here.
Up till now I see 2 issues:

1. Interface trouble
You note the UNO interface being activated.
I think I found out how that can happen, and I have a fix as a corollary of bug #59273 (needs verification over there). Bug #59273 is partly another manifestation of the same bug affecting you here.
Bug status "confirmed" applies to this issue.

2. Speed (lack of)
IIRC the sharedStrings XML can contain nested or at least very complicated strings; IIRC there was even a bug report about it 2 or 3 years ago. So simplifying the code that handles it needs proper diligence.
Note that there are also "shared formulas", we haven't implemented support for that yet but that will also rely on how we read & update sharedStrings. (Most users just use cached cell values but other interfaces allow to uncover the underlying formulas rather than their results and that doesn't fully work yet in OCT.)

I'll soon come up with a patch for the interface issues, hopefully you also want to test that for me (like the one who entered bug #59273). That issue by itself warrants a bug fix release 2.6.3 of the io package.
I'm sorry you didn't report it earlier on.

As to speed, I doubt if I have enough time to dive into it now, but that will happen. The nice thing about Octave is that it is open source so you don't need to wait until it's fixed in a next release. As I understand it you've already fixed it for yourself, hopefully if you test that a bit more I can accept it with more confidence.
It recently emerged that function calls in cell2mat work faster as follows:
cell2mat ("<function name>", <cell array>, ...)
then
cell2mat (@<function name>, <cell array., ...)
so perhaps you can shave off some more execution time.

BTW if you happen to have Excel installed on your machine, the COM interface may work up to 5 or more times faster than OCT - but for large spreadsheets it may fill up RAM very soon.
The regular expressions that OCT uses are known to be slow for very very long strings.

For really large datasets I'd advise to reconsider if you really need them in spreadsheets. Dedicated databases usually outperform any spreadsheet and because of more rigid business rules will save you from many sneaky errors commonly seen in spreadsheets like incomplete cell references, badly copied cells, dangling cell cross-references, etc etc. Not to mention the stuff that Excel and even LibreOffice do, or don't, behind your back (e.g., recalculation and cell content updates, implicit formatting affecting content formats).

Philip Nienhuis <philipnienhuis>
Group Member
Fri 16 Oct 2020 02:53:06 PM UTC, comment #6: 

As I have been really troubled by speed issues, I dived into the problem some further. I know understand why additional sheets can have impact on the performance. This is due to the way xlsx sheets are constructed. All plain strings of all worksheets are stored in one xml file. When this gets big, many cell2mat calls are made.

I have tested a very simple solution. In '__OCT_xlsx2oct__' I have replaced a single line of code, namely line 146 by the new line 147 (see attached). This line uses two cell2mat calls, and is part of a for loop. However, as far as I can tell, the list of strings that is being processed here ALWAYS contains single cells. That is why we could simply use {1}{1} instead of cell2mat(cell2mat()). This is MUCH faster.

Using the attached test script and Excel, and uncommenting line 146 (with line 147 commented) (i.e. the original code), the result is:

Elapsed time is 0.393326 seconds.
   #         Function Attr     Time (s)   Time (%)        Calls
---------------------------------------------------------------
  10         cell2mat             0.156      41.26          537
   9           regexp             0.088      23.23          285
  24 __OCT_xlsx2oct__             0.050      13.20            3
  14          cellfun             0.017       4.38         2439
   2          xls2oct             0.015       3.92            3
  36       str2double             0.009       2.35           18
  50   parse_sp_range             0.008       2.15            3
  29            fread             0.005       1.34            6
  33              cat             0.004       1.15          285
   4         prefix !             0.002       0.62         2487
  27            fopen             0.002       0.61            6
  19             sort             0.002       0.60          255
  44          col2num             0.002       0.55          742
  15              all             0.002       0.50         1384
   7           ischar             0.002       0.44         1563
  57           strrep             0.001       0.39           18
  16             size             0.001       0.38          546
  40          reshape             0.001       0.29          282
  11           nargin             0.001       0.26          559
  63           strtok             0.001       0.22            3


Commenting line 146 and uncommenting line 147 in '__OCT_xlsx2oct__' yields:

Elapsed time is 0.174529 seconds.
   #         Function Attr     Time (s)   Time (%)        Calls
---------------------------------------------------------------
   9           regexp             0.079      47.75          285
  24 __OCT_xlsx2oct__             0.028      17.02            3
   2          xls2oct             0.013       7.55            3
  36       str2double             0.008       5.07           18
  50   parse_sp_range             0.006       3.88            3
  10         cell2mat             0.006       3.77           21
  14          cellfun             0.005       3.12          117
  29            fread             0.004       2.23            6
  33              cat             0.003       2.02           21
  27            fopen             0.002       1.24            6
  44          col2num             0.002       1.05          742
   7           ischar             0.001       0.86         1563
  57           strrep             0.001       0.77           18
  63           strtok             0.001       0.51            3
  41            clear             0.000       0.28            3
  51          deblank             0.000       0.26            3
  58            index             0.000       0.24            3
  31           fclose             0.000       0.24            6
   8       chknmrange             0.000       0.23            3
  35          strncmp             0.000       0.18           12


In my real life script, which uses a bigger Excel sheet, differences are much more pronounced. As an example, using the original '__OCT_xlsx2oct__' code yields:

Elapsed time is 10.1298 seconds.
   #                             Function Attr     Time (s)   Time (%)        Calls
-----------------------------------------------------------------------------------
  81                             cell2mat             5.105      51.74        20903
 101                     __OCT_xlsx2oct__             1.249      12.66           15
  80                               regexp             1.148      11.64        10657
  29                              cellfun             0.434       4.39        94317
  73                               system             0.402       4.07            1


While the newly proposed code yields:

Elapsed time is 3.81288 seconds.
   #                             Function Attr     Time (s)   Time (%)        Calls
-----------------------------------------------------------------------------------
  80                               regexp             1.252      33.20        10657
 101                     __OCT_xlsx2oct__             0.775      20.55           15
  73                               system             0.404      10.71            1
 143               @Recipe/calc_magistral             0.169       4.48            1
  28                                 load             0.145       3.84           16


That really makes a difference, the duration went from 10s to 7s.

@philipnienhuis, can you please check that this single updated line can be implemented (i.e. the cell with strings indeed always contains a single nested cell)? If so, could you please release a new version of io as soon as possible?

NB: this solves the issue with additional sheets having an effect on speed, but it doesn't solve the issue that xls2oct sometime uses UNO even tough OCT is specified.



(file #49996, file #49997, file #49998)

Dennis <dizze>
Fri 16 Oct 2020 12:23:52 PM UTC, comment #5: 

As an example of the behavior described in comment #4, I have attached the same dummy excel file, but with a 'payload' worksheet added.

In addition, I renamed the java folders (see the pathnames mentioned in comment #3), so that the UNO interface does not work. This solves both the error, as well as the fact that xls2oct uses the UNO interface.

So, when I run the test script (comment #3) with the 'payload' sheet, the profile output is as follows:

   #         Function Attr     Time (s)   Time (%)        Calls
---------------------------------------------------------------
   9         cell2mat             0.226      43.94          537
   8           regexp             0.111      21.66          285
  23 __OCT_xlsx2oct__             0.065      12.72            3
  13          cellfun             0.025       4.95         2439
   1          xls2oct             0.021       4.03            3
  49   parse_sp_range             0.009       1.80            3
  35       str2double             0.009       1.70           18
  32              cat             0.006       1.09          285
   6           ischar             0.004       0.77         1563
  43          col2num             0.004       0.73          742
  28            fread             0.003       0.64            6
  56           strrep             0.003       0.62           18
  14              all             0.003       0.62         1384
   3         prefix !             0.003       0.60         2487
  18             sort             0.003       0.60          255
  15             size             0.002       0.45          546
  26            fopen             0.002       0.31            6
  39          reshape             0.002       0.30          282
  10           nargin             0.001       0.28          559
   5          isempty             0.001       0.26          606


When that sheet is deleted, the profile is as follows:

   #         Function Attr     Time (s)   Time (%)        Calls
---------------------------------------------------------------
   8           regexp             0.116      35.95          111
   9         cell2mat             0.095      29.62          189
  23 __OCT_xlsx2oct__             0.034      10.68            3
  13          cellfun             0.017       5.23          873
  35       str2double             0.011       3.43           18
   1          xls2oct             0.007       2.32            3
  32              cat             0.005       1.62          111
   6           ischar             0.004       1.32         1563
  28            fread             0.004       1.15            6
  56           strrep             0.003       1.08           18
  43          col2num             0.003       1.02          742
  49   parse_sp_range             0.003       0.89            3
  26            fopen             0.002       0.59            6
  14              all             0.002       0.48          514
   3         prefix !             0.002       0.47          921
  62           strtok             0.001       0.45            3
  18             sort             0.001       0.37           81
  15             size             0.001       0.30          198
  50          deblank             0.001       0.29            3
  57            index             0.001       0.27            3


The time spend by cell2mat more than halves! I don't understand what causes this, as xls2oct is called with a specific sheet. Moreover, _OCT_xlsx2oct_ only extract the specified sheet. What may cause this interference? Note: this seems to be much worse when the cell contents are plain text urls (as in the example).

To summarize, in my attempt to speed up things I have encountered two issues:
- xls2oct using UNO when OCT is specified
- the time to extract data from one worksheet is influenced by the presence (and contents) of another worksheet, especially when it contains urls

I hope these issues can be solved.

(file #49993)

Dennis <dizze>
Fri 16 Oct 2020 10:28:55 AM UTC, comment #4: 

Here is another thing that is bothering me. I was wondering why there is a huge difference between the profile results of my dummy case and the real sheet that I use. I am referring to the second call of the test script (when it uses OCT). The most time consuming function is cell2mat. But why the difference between my original sheet and the dummy?

To test this, I have stripped my real Excel sheet step by step. First removed references between sheets, removed conditional formatting, etc. That all didn't do much. What really made a big difference was removing other sheets in the workbook that contained quite a bit of text. For some reason the presence or absence of these other sheets had a huge impact on the time used by cell2mat. This seems strange to me, because the xls2oct function gets a specific worksheet as input. Why would the presence or absence of other worksheets make any difference on the time spend on cell2mat?

Dennis <dizze>
Fri 16 Oct 2020 08:01:18 AM UTC, comment #3: 

Here are the answers to your questions:

> I suspect this is partly or wholly a duplicate of bug #59273.


May not be the case, as I noticed the same behavior with v2.6.1 too.

> 1. What is the output of
> javaclasspath
> ?


>> javaclasspath

   STATIC JAVA PATH

      - empty -

   DYNAMIC JAVA PATH

      C:\Program Files\LibreOffice\program
      C:\Program Files\LibreOffice\program\classes\unoil.jar
      C:\Program Files\LibreOffice\program\classes\jurt.jar
      C:\Program Files\LibreOffice\program\classes\juh.jar
      C:\Program Files\LibreOffice\program\classes\unoloader.jar
      C:\Program Files\LibreOffice\program\classes\ridl.jar


> 2. What is the output of
> chk_spreadsheet_support ('', 3)
> ?



>> chk_spreadsheet_support ('', 3)


(OCT interface... OK, included in io package)

Checking Excel/ActiveX/COM... not working.
1. Checking Octave's Java support... OK.
2. Checking Java dependencies...
  Checking Java JRE presence.... OK, found one.
  Java seems to work OK.
  Java (version 1.8.0_261) seems OK.
  Maximum JVM memory:  1794 MiB; sufficient.
Checking javaclasspath for .jar class libraries needed for spreadsheet I/O...:

Basic POI (.xls) <poi-3> <poi-ooxml>:
  apache-poi./poi-3/poi-4....jar missing
  apache-poi-ooxml./poi-ooxml-3/poi-ooxml-4....jar missing
  => Not all classes (.jar) required for POI in classpath

POI OOXML (.xlsx) <xbean/xmlbean> <poi-ooxml-schemas> <dom4j>:
  xbean/xmlbean....jar missing
  apache-poi-ooxml-schemas/poi-ooxml-schemas....jar missing
  dom4j....jar missing
  commons-collections4....jar missing
  commons-compress-1.18....jar missing
  => Some classes for POI OOXML support missing

JExcelAPI (.xls (incl. BIFF5 read)) <jxl>:
  jxl....jar missing
  => Not all required classes (.jar) for JXL in classpath

OpenXLS (.xls - BIFF8 & .xlsx - OOXML) <OpenXLS>:
  OpenXLS....jar missing
  gwt-servlet-deps....jar missing

ODF Toolkit (.ods) <odfdom> <xercesImpl>:
  odfdom....jar missing
  xerces....jar missing
  xml-apis/xml-commons-apis....jar missing
  => Not all required classes (.jar) in classpath for OTK

jOpenDocument (.ods + experimental .sxc readonly) <jOpendocument>:
  jOpenDocument....jar missing
  => Not all required classes (.jar) for JOD in classpath

UNO/Java (.ods, .xls, .xlsx, .sxc) <OpenOffice.org>:
  - program OK
  - unoil.jar OK
  - jurt.jar OK
  - juh.jar OK
  - unoloader.jar OK
  - ridl.jar OK
  => UNO (OOo) OK
Some class libs lacking yet...

ans =  128


> 3. Exactly what commands do you use to read your spreadsheet?


I have attached a dummy example. This has exactly the same behavior: first read it uses UNO (even though I explicitly call for the "OCT" interface), second time it uses OCT and is much faster.


> Maybe it is from some of the Java interfaces or LibreOffice complaining about invalid XML? (L.O. is called when the UNO interface is invoked)


Could be the case, but I don't want it to use UNO and explicitly call for OCT.

> If you don't want UNO to be invoked you can just do
> chk_spreadsheet_support ('', -1)
>
> to wipe all Java class libs loaded by io package's initialization script from the Java class path.


This doesn't change anything. If I clear all, then use the above mention function call, and then call my script, it still uses UNO.


(file #49989, file #49990)

Dennis <dizze>
Thu 15 Oct 2020 05:57:48 PM UTC, comment #2: 

Thank for your bug report.

I suspect this is partly or wholly a duplicate of bug #59273.

But just to be sure:

1. What is the output of
javaclasspath
?

2. What is the output of
chk_spreadsheet_support ('', 3)
?

3. Exactly what commands do you use to read your spreadsheet?

The error

"CE> func=xmlSecCheckVersionExt:file=..\src\xmlsec.c:line=188:obj=unknown:subj=unknown:error=19:invalid version:mode=abi compatible;expected minor version=2;real minor version=2;expected subminor version=25;real subminor version=26"


... looks completely unfamiliar to me.
Maybe it is from some of the Java interfaces or LibreOffice complaining about invalid XML? (L.O. is called when the UNO interface is invoked)

If you don't want UNO to be invoked you can just do

chk_spreadsheet_support ('', -1)

to wipe all Java class libs loaded by io package's initialization script from the Java class path.

Philip Nienhuis <philipnienhuis>
Group Member
Thu 15 Oct 2020 04:50:12 PM UTC, comment #1: 

Forgot to mention:
- this also occured in v2.6.1
- Every first call this error is shown in the command window:

CE> func=xmlSecCheckVersionExt:file=..\src\xmlsec.c:line=188:obj=unknown:subj=unknown:error=19:invalid version:mode=abi compatible;expected minor version=2;real minor version=2;expected subminor version=25;real subminor version=26

Dennis <dizze>
Thu 15 Oct 2020 04:46:26 PM UTC, original submission:  

In a script that uses many reads (no writes) from an Excel sheet, I am trying to optimize for speed. For that reason I do not use xlsread, but xlsopen/xls2oct/xlsclose. I call xlsopen using "OCT" as the defined interface.

I noticed that every first time a sheet is read, it takes a long time. Using the profiler I found out that this first time, the xls functions seem to use the UNO interface.

After the first run, the profile looks like this:
   #                             Function Attr     Time (s)   Time (%)        Calls
-----------------------------------------------------------------------------------
  59                     _UNO_spsh2oct_             8.068      90.04            3
  54                             cell2mat             0.355       3.97          892
   1 @Recipe/read_magistral_relationships             0.105       1.17            1
  62                             strsplit             0.066       0.74          105
  74                       parse_sp_range             0.050       0.55           99
  72                 _UNO_getusedrange_             0.042       0.47            3
  83                               strtok             0.035       0.39           99
  23                              cellfun             0.024       0.26         3680
  81                                index             0.019       0.21           99
  22                                 load             0.018       0.21            2
  75                              deblank             0.018       0.20           99
  53                               regexp             0.016       0.17          306
  50                              xls2oct             0.015       0.17            3
  21                   installed_packages             0.012       0.13            1
  37                                 sort             0.009       0.10          711
  63                          parseparams             0.009       0.10          105
  56                                  all             0.008       0.09         2123
   8                             prefix !             0.007       0.08         4633
  61                           javaObject             0.006       0.07          193
  13                              isempty             0.005       0.06         2568

The second time, it looks like this:
   #                             Function Attr     Time (s)   Time (%)        Calls
-----------------------------------------------------------------------------------
  54                             cell2mat             2.121      55.11         5152
  53                               regexp             0.638      16.59         2148
  60                     _OCT_xlsx2oct_             0.408      10.59            3
  23                              cellfun             0.201       5.22        22781
   1 @Recipe/read_magistral_relationships             0.096       2.50            1
  70                           str2double             0.039       1.02           18
  37                                 sort             0.033       0.86         2820
   8                             prefix !             0.033       0.85        22819
  67                                  cat             0.031       0.79         2354
  56                                  all             0.030       0.78        12776
  75                              col2num             0.022       0.57         3428
  49                                 size             0.021       0.55         5210
  28                               ischar             0.021       0.55        10630
  50                              xls2oct             0.019       0.49            3
  55                               iscell             0.016       0.43         5158
  22                                 load             0.016       0.41            2
  72                              reshape             0.013       0.34         2333
   4                               nargin             0.013       0.33         5179
  13                              isempty             0.012       0.32         6022
  21                   installed_packages             0.012       0.32            1

Note that the function '__UNO_spsh2oct__' no longer appears in the list. This second time is much faster as well.

Note 1: after 'clear functions' the profile looks like the first.
Note 2: I am using an xlsx spreadsheet. No clue why UNO is called.
Note 3: I use io package version 2.6.2

Dennis <dizze>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #50099:  __OCT_xlsx2oct__.m added by philipnienhuis (10KiB - application/octet-stream)
file #50093:  Excel2019@stringTest1.xlsx added by dizze (83KiB - application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
file #50094:  Test2.m added by dizze (274B - application/octet-stream)
file #50077:  __OCT_xlsx2oct__.m added by philipnienhuis (10KiB - application/octet-stream)
file #50026:  __OCT_xlsx2oct__.m added by dizze (10KiB - application/octet-stream)
file #50017:  Excel2019@stringTest1.xlsx added by dizze (74KiB - application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
file #50018:  __OCT_xlsx2oct__.m added by dizze (10KiB - application/octet-stream)
file #50019:  LibreOffice6@stringTest1.xlsx added by dizze (129KiB - application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
file #50020:  Excel2019@stringTest2.xlsx added by dizze (12KiB - application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
file #49996:  __OCT_xlsx2oct__.m added by dizze (10KiB - application/octet-stream)
file #49997:  BLAAAAA.m added by dizze (599B - application/octet-stream)
file #49998:  test.xlsx added by dizze (65KiB - application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
file #49993:  test.xlsx added by dizze (65KiB - application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
file #49989:  test.xlsx added by dizze (44KiB - application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
file #49990:  BLAAAAA.m added by dizze (571B - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by siko1056 (Updated the item)
  • -email is unavailable- added by dizze (Submitted the item)
  • -email is unavailable- added by dizze (This concerns the IO package)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 25 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-04-28 siko1056 Summary[octave-forge](io) xls2oct is slow when a spreadsheet contains many text cells [octave forge] (io) xls2oct is slow when a spreadsheet contains many text cells
    2020-10-25 philipnienhuis Attached File- Added _OCT_xlsx2oct_.m, #50099
        StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2020-10-25 dizze Attached File- Added Excel2019@stringTest1.xlsx, #50093
        Attached File- Added Test2.m, #50094
    2020-10-24 philipnienhuis Summaryxls2oct and/or openxls behave unexpected [octave-forge](io) xls2oct is slow when a spreadsheet contains many text cells
    2020-10-23 philipnienhuis StatusConfirmed In Progress
    2020-10-23 philipnienhuis Attached File- Added _OCT_xlsx2oct_.m, #50077
    2020-10-20 dizze Attached File- Added _OCT_xlsx2oct_.m, #50026
    2020-10-19 dizze Attached File- Added Excel2019@stringTest1.xlsx, #50017
        Attached File- Added _OCT_xlsx2oct_.m, #50018
        Attached File- Added LibreOffice6@stringTest1.xlsx, #50019
        Attached File- Added Excel2019@stringTest2.xlsx, #50020
    2020-10-18 philipnienhuis StatusNeed Info Confirmed
    2020-10-18 philipnienhuis StatusConfirmed Need Info
    2020-10-16 philipnienhuis StatusNeed Info Confirmed
        Assigned toNone philipnienhuis
        Release5.2.0 other
        Operating SystemMicrosoft Windows Any
    2020-10-16 dizze Attached File- Added _OCT_xlsx2oct_.m, #49996
        Attached File- Added BLAAAAA.m, #49997
        Attached File- Added test.xlsx, #49998
    2020-10-16 dizze Attached File- Added test.xlsx, #49993
    2020-10-16 dizze Attached File- Added test.xlsx, #49989

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code