Do not ignore error in os_free() when size == 0 #482
Conversation
ldorau
force-pushed
the
Do_not_ignore_error_in_os_free_when_size_==_0
branch
from
786689a to
93f94a9
Compare
ldorau
changed the title
ldorau
force-pushed
the
Do_not_ignore_error_in_os_free_when_size_==_0
branch
from
93f94a9 to
2bbd9fc
Compare
benchmark/ubench.c
Outdated
| @@ -196,6 +196,7 @@ UBENCH_EX(simple, proxy_pool_with_os_memory_provider) { | |||
| } | |||
|
|
|||
| umf_memory_pool_handle_t proxy_pool; | |||
| // the proxy pool requires the tracking provider to work correctly | |||
There was a problem hiding this comment.
The tracking provider is always required to work correctly :)
Would you like to say that tracking capabilities should be ON?
My general thoughts: the amount of features that depend on tracking capabilities is increasing. We should consider making the tracking provider always ON (remove the option that allows to disable it) or we need to check it at build/run time and return appropriate error. I vote for the first option (always unconditional ON).
benchmark/ubench.c
Outdated
| @@ -238,8 +239,10 @@ UBENCH_EX(simple, disjoint_pool_with_os_memory_provider) { | |||
| disjoint_memory_pool_params.MinBucketSize = DISJOINT_POOL_MIN_BUCKET_SIZE; | |||
|
|
|||
| umf_memory_pool_handle_t disjoint_pool; | |||
| umf_result = umfPoolCreate(umfDisjointPoolOps(), os_memory_provider, | |||
| &disjoint_memory_pool_params, 0, &disjoint_pool); | |||
| // disable the tracking provider to make it work faster (UMF_POOL_CREATE_FLAG_DISABLE_TRACKING) | |||
There was a problem hiding this comment.
Are we interested in performance without a tracking provider? But in real workloads tracking capabilities will be ON?
There was a problem hiding this comment.
Right. Removed.
Done.
| @@ -343,6 +348,15 @@ static umf_result_t trackingFree(void *hProvider, void *ptr, size_t size) { | |||
| umf_ba_global_free(value); | |||
| } | |||
|
|
|||
| // umfMemoryProviderFree() should not be called with size == 0, | |||
There was a problem hiding this comment.
But according to this PR umfMemoryProviderFree() for tracking provider can be called with size == 0.
Maybe a particular pool implementation should explicitly get the size from the tracker and call umfMemoryProviderFree() with the right size?
There was a problem hiding this comment.
Right. New fix: #487
Signed-off-by: Lukasz Dorau <[email protected]>
umfMemoryProviderFree() should not be called with size == 0, so use the size saved in the tracking provider in this case. Signed-off-by: Lukasz Dorau <[email protected]>
Ref: oneapi-src#475 Ref: oneapi-src#481 Signed-off-by: Lukasz Dorau <[email protected]>
ldorau
force-pushed
the
Do_not_ignore_error_in_os_free_when_size_==_0
branch
from
2bbd9fc to
657b704
Compare
ldorau
changed the title
|
Replaced with #487 |
Description
Do not ignore error in
os_free()when size == 0.Ref: #481
Checklist