Skip to content
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

Remove memory scope and improve memory management #695

Merged
merged 1 commit into from
Mar 9, 2021

Commits on Mar 9, 2021

  1. Remove memory scope and improve memory management

    The MemoryScope reveals a number of shortcomings within the DJL memory
    management. While the MemoryScope is deleted, many of them are fixed as part of
    this PR.
    
    First, the NDManager.{attach, detach} were renamed to xxxInternal. This is to
    differentiate them from the attach and detach methods that are intended to be used.
    
    There are two new concepts in memory management. An NDResource interface was
    created to combine the concepts of managed memory that was used in NDArray and
    NDList. It could also be used in more classes in the future. This includes the
    getManager, attach, and detach.
    
    Within the NDManager, it gains a second "management convention". The first
    convention of normal resources are added to the manager and then closed when the
    manager closes. This works for small numbers of things on the NDArray, but not
    when operations transitively create. So, the second convention is a
    tempResource. Instead of freeing them when the manager is closed, they are
    returned to their original manager. This is used to create a temporary scope, do
    operations within it, and then the inputs and return value are returned to the
    parent while the intermediate work is cleaned. This also matches the concepts of
    ownership/borrowing as well.
    
    Using these, a few additional helper methods were created. There is
    `NDManager.from(resource)` to ease creation of managers based on a resource.
    There is also `scopeManager.ret(returnValue)` to help with returning values
    outside of the scopeManager. Lastly, there is a `scopeManager.{temp,}AttachAll`
    to attach a number of resources to a manager within a single call.
    
    Using these improvements, the new method were applied to the old locations where
    MemoryScope was used as well as an additional case in NDManagerEx.
    
    Also, the old attach methods were altered to be `void`. Because the return
    values are no longer used anywhere and are not as necessary in the current
    scheme, I figured it would simplify things. It also helps for things like
    `NDList.attach` which does not have a single original NDManager when attaching.
    
    Change-Id: I91d109cd14d70fa64fd8fffa0b50d88ab053013e
    zachgk committed Mar 9, 2021
    Configuration menu
    Copy the full SHA
    ac461c0 View commit details
    Browse the repository at this point in the history