xarray 2022.10.0 much slower then 2022.6.0 #7181
Comments
dschwoerer
added
the
needs triage
|
Here's the output of With xarray 2022.10 (re-read the xarray 2022.06: It looks like it's basically all coming from copying dataarrays -- has there been some change in expected behaviour that we need to update our code? * |
|
Yes the behavior of deep copy was changed. Maybe there is some bug in the logic, or maybe before it was not doing true deep copies? |
TomNicholas
added
topic-performance
regression
and removed
needs triage
|
Our tests all pass, but there is something like a x20 slowdown, and it's basically entirely due to copies. It's plausible we're doing way too many copies as it is, but this is obviously still concerning. I tried adding the following asv benchmark, based off the class Copy:
def setup(self):
"""Create 4 datasets with two different variables"""
t_size, x_size, y_size = 50, 450, 400
t = np.arange(t_size)
data = np.random.randn(t_size, x_size, y_size)
self.ds = xr.Dataset(
{"A": xr.DataArray(data, coords={"T": t}, dims=("T", "X", "Y"))}
)
def time_copy(self) -> None:
copy(self.ds)
def time_deepcopy(self) -> None:
deepcopy(self.ds)
def time_copy_method(self) -> None:
self.ds.copy(deep=False)
def time_copy_method_deep(self) -> None:
self.ds.copy(deep=True)But I didn't see any regressions between There are a few differences between our test datasets and the one in the benchmark above:
|
|
the only difference you should see is in the deep-copy of |
|
Nope, either single values or flat dicts: In [6]: ds.n
Out[6]:
<xarray.DataArray 'n' (t: 6, x: 6, y: 12, z: 7)>
dask.array<concatenate, shape=(6, 6, 12, 7), dtype=float64, chunksize=(6, 2, 4, 7), chunktype=numpy.ndarray>
Coordinates:
dx (x, y) float64 dask.array<chunksize=(2, 4), meta=np.ndarray>
dy (x, y) float64 dask.array<chunksize=(2, 4), meta=np.ndarray>
dz float64 0.8976
* t (t) float64 0.0 10.0 20.0 30.0 40.0 50.0
* x (x) int64 0 1 2 3 4 5
* y (y) float64 1.0 3.0 5.0 7.0 9.0 11.0 13.0 15.0 17.0 19.0 21.0 23.0
* z (z) float64 0.0 0.8976 1.795 2.693 3.59 4.488 5.386
Attributes:
direction_y: Standard
cell_location: CELL_CENTRE
direction_z: Standard
metadata: {'BOUT_VERSION': 4.3, 'use_metric_3d': 0, 'NXPE': 3, 'NYP...
options: None
geometry:Our coordinates are |
|
You call |
|
A call graph was posted in the referenced thread: boutproject/xBOUT#252 (comment) |
|
I see, I think the problem is |
|
@dschwoerer could you test if you see any improvement with this PR: #7209 ? The graph is not super helpfull, since most calls to deep_copy are indirectly from deep_copy itself. I tried my best to find some points of improvement. |
|
@headtr1ck It does seem to have helped a bit, but looks like it's still around x15 slower: I think previously |
|
Indeed, it does help. In 6 hours the CI completed 50% of the tests, compared to 17%. This however still very much slower than before, where we finished in around half an hour - so around 24x slower ... |
|
Looking at the call tree (sorry, big image!):
there's a couple of recursive calls that have all the time:
Is this implying we do actually have some sort of complex or recursive structure hiding in our datasets? |
|
Every time I look at this graph Im getting more confused, haha. |
|
You call But then somehow There are some copy calls from alignments, but I don't see how that adds up to this number... |
I think it has to be something like that. I'll see if I can dig into what these Datasets look like. I'm sure we can get away with fewer deep copies here. I think it's something like we're caching generated Datasets between different test cases, but some of them need to modify them so they're deep copied. I guess previously that was fine, but maybe now it's just cheaper to make new instances from scratch. |
|
Yes, it turns out we do have Datasets with DataArrays in attributes. They're not nested very deep, and they're not cyclic, but it looks like there might be two or three levels of nesting. Is this nesting now disallowed? |
No, it is allowed. |
What is your issue?
xbout's test suite finishes with 2022.6.0 in less than an our, with 2022.10.0 it gets aborted after 6 hours.
I haven't managed to debug what is the issue.
Git bisect will not work, as 2022.9.0 is broken due to #7111
The text was updated successfully, but these errors were encountered: