ENH: optimize import time #199
Conversation
|
One way to check if this has real impacts on a codebase that depends on unyt is to run the yt unit tests and look for new failures. I can think of cases where shallow copying the unit registry dict could affect runtime behavior, particularly in situations where there’s more than one unit registry to worry about. yt exercises those cases much more than the unyt tests. |
|
Good call, thank you ! |
|
I've run yt's test suite against this branch and everything stayed green ! |
neutrinoceros
force-pushed
the
opti_import_time
branch
from
9818fa1
to
d5dbd06
Compare
|
On my other machine the import time goes from 2.4s to 1.8s. Still about 25% gain. about 50% of the remaining import time is from unyt itself so I expect there is still room for future optimisations. |
|
@neutrinoceros please rebase this and perform the same check on the yt codebase that Nathan suggested earlier again, since it's been so long. |
|
Just did. Still no error to report ! |
|
@matthewturk could you double-check this one? |
|
@neutrinoceros please fix this one, we'll get it in for 2.9 |
|
Sure ! I can do that in a few hours, when I'm home. |
neutrinoceros
force-pushed
the
opti_import_time
branch
from
d5dbd06
to
20ff3a2
Compare
|
@jzuhone done |
|
@matthewturk Could you have a quick look at this? |
|
FWIW I'm working on automating the process of checkin compatibility with unyt's dev branch on yt's side here yt-project/yt#4007 |
|
This looks OK to me. I think that one of the things we should look at is how the change in turning off deep-copy-by-default affects performance -- a while back I remember looking at things like covering grids etc in yt and finding that much of the time was spent in copying and creating the affiliated objects (like the various |
|
So, would a simple benchmark using yt's covering grid do ? |
|
@neutrinoceros We do not need to run one. I was noting it because I want to run one after thigns have settled down. |
|
So are we go on this ? |
This is an answer to #27. I shave off about 33% of unyt's import time by making Unit objects copies shallow by default, the one difference with a deep copy being that the attached UnitRegistry is shallow-copied.
Using the benchmark I described in #27, I get the import time from 1.6s to 1.0s on my machine.
I hope that this doesn't have undesirable side effects.
Another aspect that could be considered is that the sheer number of copies that are performed at import time is probably a sign that something else isn't optimized.