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

Generalize ur_usm_pool_limits_desc_t #369

Open
igchor opened this issue Mar 21, 2023 · 2 comments
Open

Generalize ur_usm_pool_limits_desc_t #369

igchor opened this issue Mar 21, 2023 · 2 comments
Labels
needs-discussion This needs further discussion specification Changes or additions to the specification

Comments

@igchor
Copy link
Member

igchor commented Mar 21, 2023

Currently, ur_usm_pool_limits_desc_t exposes parameters similar to the ones exposed by SYCL_PI_LEVEL_ZERO_USM_ALLOCATOR .

There are a few problems with existing params:

  1. capacity is tricky to use: it describes a number of allocations and does not take into account allocation size. Also, UR does not expose any statistics that could be used to tweak the capacity. Moreover, it might not be applicable to all heap managers.

    I propose removing this parameter.

  2. maxPoolSize might be ambiguous. Unified Runtime can create multiple 'sub-pools' - it can decide to create pool per each device or to create a pool that spans multiple devices in certain cases. Having the same pool size limit for both of those cases will not be optimal.

    We could replace the maxPoolSize with a decay parameter that, instead of specifying limits in terms of space, defines it in terms of time - for how long the pool is allowed to keep memory pages around, similar to: https://jemalloc.net/jemalloc.3.html#arenas.dirty_decay_ms

Related discussion: #324

@igchor igchor added needs-discussion This needs further discussion specification Changes or additions to the specification labels Mar 21, 2023
@RaviNarayanaswamy
Copy link

  • Capacity allows user to allocate memory suited for his program without going to level0 each time memory is requested. This is used in spec
  • I am not sure if maxPoolSize is useful, I assume you have maxAllocSize which says anything bigger than this should not be pooled.

@igchor
Copy link
Member Author

igchor commented Mar 22, 2023

  • Capacity allows user to allocate memory suited for his program without going to level0 each time memory is requested. This is used in spec
  • I am not sure if maxPoolSize is useful, I assume you have maxAllocSize which says anything bigger than this should not be pooled.

The problem with capacity is that, I believe, it does not take allocation size into account. Distribution of allocation sizes is usually not flat.

In OpenMP there is both AllocMax (aka maxAllocSize) and poolSize (aka maxPoolSize). We definitely want to keep AllocMax but as you said, poolSize/maxPoolSize might not be useful.

Ref: https://www.intel.com/content/www/us/en/docs/dpcpp-cpp-compiler/developer-guide-reference/2023-0/supported-environment-variables.html

igchor added a commit to igchor/unified-runtime that referenced this issue Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-discussion This needs further discussion specification Changes or additions to the specification
Projects
None yet
Development

No branches or pull requests

2 participants