-
Notifications
You must be signed in to change notification settings - Fork 262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Potential N^2 issue in large file read #1279
Comments
A correction to the above -- debug nccopy time for netcdf-3 is 0.7 to 0.8 seconds. Optimized shows:
|
Can you try with the code in PR #1262. Can you try with that code? You must recreate you test file with the library after applying 1262. Then you will avoid the dimscale mathing. |
I tried it on my laptop (not the system used above) and it only dropped times from about 10.6 to 9.9. I will try on the system above later this week or early next week. |
Part of the problem is that nccopy is in the end going to |
Actually I see there is an h5copy utility. Might be interesting to |
Greg, did you recreate your file with the new version of netcdf? Use h5dump to confirm that all datasets in the file have the _Netcdf4Coodinates attr, which lists dimids for that dataset. |
Using valgrind, the patch reduced the overall execution time from 18.289 to 15.364 or 16% which is good. I haven't found a good hdf5 utility to check how fast it can read/write the files. |
Can you use a profiler and see where all the time is being spent? It should not be in rec_match_dimscales(). So where is it? |
The percentage time in I've attached a screen shot showing the before and after profile sorted by I recreated the file using |
It looks like the |
If I modify to write the (I think the |
What do you mean "modify to write the _Netcdf4Coordinates attribute for all variables"? This should be happening automatically. |
No, currently only happening for multi-dimensional variables according to documentation and to code. See
|
OK, thanks for catching that, it should be used even for 1D vars. I will fix that. Let me construct a test case with 5000 dimensions and see what I can see. ;-) |
I got h5copy to copy the file using this command: |
If nclistsetalloc is the problem, then it is likely because its default list |
I am modifying the recently-added tst_wrf_reads.c to emulate this case. That way we can try a few things... |
`nclistsetalloc` isn't the main problem, it just seemed higher than it
should be. I don't think it doubles every time. I was seeing many
instances in which it only increased in size by 1 each time... Might be
good too look closer. It seems to only double if the `sz` argument coming
in is <= 0.
…On Mon, Jan 14, 2019 at 1:51 PM Ed Hartnett ***@***.***> wrote:
I am modifying the recently-added tst_wrf_reads.c to emulate this case.
That way we can try a few things...
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1279 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA2xDoXjI3SObSBGFryk99fH6QVWBso0ks5vDO3ggaJpZM4Z4XKZ>
.
|
oops, you are correct. It doubles only if no size is specified, otherwise |
See discussion in Unidata#1279
To be precise,
ndims is rank of each var, so it is not O(nvars * alldims) but rather much less. |
Once your changes in #2047 are merged, this should improve a lot... |
@DennisHeimbigner I disagree, without the coord dimids on the 1D variables, the behavior scales as I think it actually scales as O( (1D nvars) * ndims) |
Odd. I was looking at the code fore rec_match_dimscales and it |
I think that if it has the dim ids associated with the variable, then it can do the direct lookup, but if it is a 1D variable, then it has to search all of the dims for the dimid. That is the speedup of #2407 is that it gives us the coordinate dimid attribute associated with all variables and then we do get the direct lookup instead of the current behavior without the patch where 1D variables do not have any way of storing the dimid and therefore there isn't a direct lookup. Maybe I am misunderstanding your question/assertion. |
If you have the NC_VAR_INFO_T* pointer to the var info, |
IIRC rec_match_dimscales should be called only when our own hidden attribute metadata does not know the dimids. In that case, rec_match_dimscales() does a very slow recursive search through the file for matching dimscales. Since something like 4.6.x the dim info is always written to our internal metadata hidden attributes, but before that, it was not. |
Environment Information
configure
)C
code to recreate the issue?Summary of Issue
I have a file with 10,000 dims and 7,000 vars. If I do a
nccopy
with the file in netcdf-3 classic format, it takes 0.422 seconds. If I do the samenccopy
except that the file is in netcdf-4 classic format, it takes 13.360 seconds. These are both for debug, non-optimized builds.I ran the
nccopy
in valgrind and it points to therec_match_dimscales
function as taking 32.68 % of the time (inclusive -- including the functions it calls). It callsncindexth
andnclistget
each about 60,000,000 times.It looks like
rec_match_dimscales
has a loop over the vars and then inside that loop it loops over the dims to find matches for thedimscales
of the var. This results in basicallyO(nvars * ndims)
scale behavior.Maybe it is possible to somehow preprocess the
dims
data to make it easier to match the vars dimscale dims without a linear search through thedims
list? Perhaps a hash on theobjno
andfileno
would permit O(1) access instead of O(N^2)...The attached file was compressed with gzip.
mix-nc4.g.gz
These are all with current github master.
The text was updated successfully, but these errors were encountered: