[UMF] Bump version and update DisjointPool params handling #2426
Conversation
| Configs[DisjointPoolMemType::Device].Name = "Device"; | ||
| Configs[DisjointPoolMemType::Shared].Name = "Shared"; | ||
| Configs[DisjointPoolMemType::SharedReadOnly].Name = "SharedReadOnly"; | ||
| ret = umfDisjointPoolParamsSetName(Configs[DisjointPoolMemType::Host], "Host"); |
There was a problem hiding this comment.
nit: you can just do CheckConfigRet(umfDisjointPoolParamsSetName(Configs[DisjointPoolMemType::Host], "Host")).
Also, I would rename CheckConfigRet to UMF_CALL or something like that so it's shorter and more consistent with UR_CALL/ZE_CALL
| for (auto &Config : AllConfigs.Configs) { | ||
| Config.MaxPoolableSize = AllConfigs.Configs[LM].MaxPoolableSize; | ||
| ret = umfDisjointPoolParamsSetMaxPoolableSize(Config, TmpValue); | ||
| CheckConfigRet(ret); |
There was a problem hiding this comment.
This return will only exit from this lambda, not from the entire parseDisjointPoolConfig function.
| Config.SharedLimits = AllConfigs.limits.get(); | ||
| Config.PoolTrace = trace; | ||
| umfDisjointPoolParamsSetSharedLimits(Config, AllConfigs.limits.get()); | ||
| umfDisjointPoolParamsSetTrace(Config, trace); |
| #define CheckConfigRet(umf_ret) \ | ||
| if (umf_ret != UMF_RESULT_SUCCESS) { \ | ||
| logger::error("DisjointPool params failed"); \ | ||
| return; \ |
There was a problem hiding this comment.
Should we just ignore the error? If any of the setParams function fail and we will just exit the function we might use incomplete config later on.
For calls in DisjointPoolAllConfigs::DisjointPoolAllConfigs I think we should have an assert and for calls in parseDisjointPoolConfig if there is any error we should catch it, log it, and probably revert all configs to the default settings.
lukaszstolarczuk
force-pushed
the
bump-umf-with-configs
branch
from
7da67f9 to
6479328
Compare
| << std::setw(12) | ||
| << AllConfigs.Configs[DisjointPoolMemType::SharedReadOnly].Capacity | ||
| << std::endl; | ||
| // TODO: fixme, accessing config values directly is no longer allowed - API's changed |
There was a problem hiding this comment.
I tried to do this nicely, but I'm not sure if I can... @igchor tried to make the pre-generated content in ctor, but it would have to be updated somehow in the parsing function... If this is not crucial I could update that in a sep. PR...?
It includes update in the new params config approach.
lukaszstolarczuk
force-pushed
the
bump-umf-with-configs
branch
from
6479328 to
2cb639e
Compare
github-actions
bot
added
ci/cd
lukaszstolarczuk
force-pushed
the
bump-umf-with-configs
branch
from
2cb639e to
7d08ec1
Compare
|
replaced with #2436 |
2 participants
using UMF with PR oneapi-src/unified-memory-framework#969