Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Optimization: reduce number of deepcopies #477

Closed
omry opened this issue Jan 19, 2021 · 2 comments · Fixed by #481
Closed

Optimization: reduce number of deepcopies #477

omry opened this issue Jan 19, 2021 · 2 comments · Fixed by #481
Milestone

Comments

@omry
Copy link
Owner

omry commented Jan 19, 2021

During assignment of any value to an OmegaConf container, it is deep-copied.
the motivation is primarily address the case a node from an existing config tree is assigned to another config:

    cfg1 = OmegaConf.create({"foo": {"bar": 10}})
    cfg2 = OmegaConf.create({})
    cfg2.foo = cfg1.foo
    assert id(cfg1.foo._get_parent()) == id(cfg1)
    assert id(cfg2.foo._get_parent()) == id(cfg2)

Without a deepcopy, the parent of the node cfg1.foo will change and cfg1 will be broken and the asserts will fail (resulting in interpolation and other issues).

Deepcopy is a clean solution, but it's expensive and most cases an overkill.
to improve initialization performance, we can limit deepcopy only to the rare cases where a node that belongs to an existing OmegaConf container is assigned.

This is a potentially breaking change in rare cases but think those are unlikely.

@o-smirnov
Copy link

Thanks for this -- it's made a huge difference -- my config load times dropped from 10s to <0.5s.

@omry
Copy link
Owner Author

omry commented Feb 14, 2021

Great to hear. There are some other optimization in 2.1 as well.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
2 participants