Use base allocator in the tracking provider #170

ldorau · 2024-01-24T13:59:24Z

No description provided.

src/provider/provider_tracking.c

igchor · 2024-01-24T16:50:57Z

Just FYI (it doesn't need to be in this PR) we also need to use ba_allocator in critnib to avoid the issues with jemalloc.

src/provider/provider_tracking.c

ldorau · 2024-01-26T09:30:16Z

Just FYI (it doesn't need to be in this PR) we also need to use ba_allocator in critnib to avoid the issues with jemalloc.

Done: #178

src/provider/provider_tracking.h

igchor · 2024-01-29T16:04:35Z

@ldorau do you think we could create a simple benchmark for memory tracker so that we could see how the performance looks like after applying #170 and #178? Memory tracking is already used in UR so I wouldn't want to introduce a regression.

ldorau · 2024-01-29T17:19:39Z

@ldorau do you think we could create a simple benchmark for memory tracker so that we could see how the performance looks like after applying #170 and #178? Memory tracking is already used in UR so I wouldn't want to introduce a regression.

What API do you want to benchmark ?

ldorau · 2024-01-29T21:56:55Z

@ldorau do you think we could create a simple benchmark for memory tracker so that we could see how the performance looks like after applying #170 and #178? Memory tracking is already used in UR so I wouldn't want to introduce a regression.

@igchor Benchmark on the main branch (UMF_ENABLE_POOL_TRACKING==ON):

[ RUN      ] simple.os_memory_provider
[       OK ] simple.os_memory_provider (mean 1.021ms, confidence interval +- 1.597071%)

with base allocator (UMF_ENABLE_POOL_TRACKING==ON):

[ RUN      ] simple.os_memory_provider
[       OK ] simple.os_memory_provider (mean 976.713us, confidence interval +- 1.603028%)

Is it enough?

igchor · 2024-01-29T22:00:53Z

@ldorau do you think we could create a simple benchmark for memory tracker so that we could see how the performance looks like after applying #170 and #178? Memory tracking is already used in UR so I wouldn't want to introduce a regression.

@igchor Benchmark on the main branch (UMF_ENABLE_POOL_TRACKING==ON):
[ RUN      ] simple.os_memory_provider
[       OK ] simple.os_memory_provider (mean 1.021ms, confidence interval +- 1.597071%)
with base allocator (UMF_ENABLE_POOL_TRACKING==ON):
[ RUN      ] simple.os_memory_provider
[       OK ] simple.os_memory_provider (mean 976.713us, confidence interval +- 1.603028%)
Is it enough?

I don't think the os_memory_provider benchmark does any memory tracking (tracking is only enabled when there is a memory pool). I think what we should implement is a benchmark with a 'proxy pool' - that way we can reuse the existing code and we won't have any impact from an actual pool implementation. Also, UR does use 'proxy pool' for certain scenarios.

All issues fixed

ldorau · 2024-02-01T14:49:53Z

I don't think the os_memory_provider benchmark does any memory tracking (tracking is only enabled when there is a memory pool). I think what we should implement is a benchmark with a 'proxy pool' - that way we can reuse the existing code and we won't have any impact from an actual pool implementation. Also, UR does use 'proxy pool' for certain scenarios.

@igchor
Results of benchmark of proxy pool WITHOUT the base allocator in the tracking provider:

[==========] Running 3 benchmarks.
[ RUN      ] simple.glibc_malloc
[       OK ] simple.glibc_malloc (mean 1.774ms, confidence interval +- 0.671806%)
[ RUN      ] simple.os_memory_provider
[       OK ] simple.os_memory_provider (mean 992.191us, confidence interval +- 1.602488%)
[ RUN      ] simple.proxy_pool_with_os_memory_provider
[       OK ] simple.proxy_pool_with_os_memory_provider (mean 738.283us, confidence interval +- 0.920939%)
[==========] 3 benchmarks ran.
[  PASSED  ] 3 benchmarks.

Results of benchmark of proxy pool WITH the base allocator in the tracking provider (see https://github.com/ldorau/unified-memory-framework/tree/base_alloc):

[==========] Running 3 benchmarks.
[ RUN      ] simple.glibc_malloc
[       OK ] simple.glibc_malloc (mean 1.808ms, confidence interval +- 0.688160%)
[ RUN      ] simple.os_memory_provider
[       OK ] simple.os_memory_provider (mean 989.281us, confidence interval +- 1.545102%)
[ RUN      ] simple.proxy_pool_with_os_memory_provider
[       OK ] simple.proxy_pool_with_os_memory_provider (mean 728.101us, confidence interval +- 1.210658%)
[==========] 3 benchmarks ran.
[  PASSED  ] 3 benchmarks.

(see #198)

igchor · 2024-02-01T17:09:13Z

src/provider/provider_tracking.h

-    if (!handle->map) {
-        free(handle);
-        return NULL;
+    umf_ba_pool_t *pool_tracker = umf_ba_create(sizeof(struct tracker_value_t));


nit: pool_tracker sounds like it's something that tracks pools ;d Can we rename it to tracker_allocator?

igchor · 2024-02-01T17:09:49Z

I don't think the os_memory_provider benchmark does any memory tracking (tracking is only enabled when there is a memory pool). I think what we should implement is a benchmark with a 'proxy pool' - that way we can reuse the existing code and we won't have any impact from an actual pool implementation. Also, UR does use 'proxy pool' for certain scenarios.

@igchor Results of benchmark of proxy pool WITHOUT the base allocator in the tracking provider:
[==========] Running 3 benchmarks.
[ RUN      ] simple.glibc_malloc
[       OK ] simple.glibc_malloc (mean 1.774ms, confidence interval +- 0.671806%)
[ RUN      ] simple.os_memory_provider
[       OK ] simple.os_memory_provider (mean 992.191us, confidence interval +- 1.602488%)
[ RUN      ] simple.proxy_pool_with_os_memory_provider
[       OK ] simple.proxy_pool_with_os_memory_provider (mean 738.283us, confidence interval +- 0.920939%)
[==========] 3 benchmarks ran.
[  PASSED  ] 3 benchmarks.
Results of benchmark of proxy pool WITH the base allocator in the tracking provider (see https://github.com/ldorau/unified-memory-framework/tree/base_alloc):
[==========] Running 3 benchmarks.
[ RUN      ] simple.glibc_malloc
[       OK ] simple.glibc_malloc (mean 1.808ms, confidence interval +- 0.688160%)
[ RUN      ] simple.os_memory_provider
[       OK ] simple.os_memory_provider (mean 989.281us, confidence interval +- 1.545102%)
[ RUN      ] simple.proxy_pool_with_os_memory_provider
[       OK ] simple.proxy_pool_with_os_memory_provider (mean 728.101us, confidence interval +- 1.210658%)
[==========] 3 benchmarks ran.
[  PASSED  ] 3 benchmarks.
(see #198)

Great. It would be good to also have multithreaded benchmark, but I think this is good enough for now.

ldorau · 2024-02-01T21:25:26Z

nit: pool_tracker sounds like it's something that tracks pools ;d Can we rename it to tracker_allocator?

Renamed

ldorau · 2024-02-01T21:26:08Z

@bratpiorka review please :-)

src/provider/provider_tracking.c

Signed-off-by: Lukasz Dorau <[email protected]>

igchor reviewed Jan 24, 2024

View reviewed changes

src/provider/provider_tracking.c Outdated Show resolved Hide resolved

ldorau force-pushed the Use_base_allocator_in_the_tracking_provider branch 2 times, most recently from 995c6bb to 8c8e024 Compare January 24, 2024 17:21

bratpiorka previously requested changes Jan 24, 2024

View reviewed changes

src/provider/provider_tracking.c Outdated Show resolved Hide resolved

ldorau force-pushed the Use_base_allocator_in_the_tracking_provider branch 7 times, most recently from bcbb979 to 9d7e19d Compare January 25, 2024 22:32

ldorau mentioned this pull request Jan 26, 2024

Add linear base allocator #177

Merged

ldorau force-pushed the Use_base_allocator_in_the_tracking_provider branch 2 times, most recently from 4b0e902 to 7ccda59 Compare January 26, 2024 10:55

ldorau marked this pull request as ready for review January 26, 2024 13:28

ldorau requested a review from a team as a code owner January 26, 2024 13:28

ldorau changed the title ~~[DRAFT] Use base allocator in the tracking provider~~ Use base allocator in the tracking provider Jan 26, 2024

ldorau requested review from bratpiorka and igchor January 26, 2024 13:29

ldorau force-pushed the Use_base_allocator_in_the_tracking_provider branch 2 times, most recently from fba26dc to 480eaa7 Compare January 29, 2024 12:00

igchor reviewed Jan 29, 2024

View reviewed changes

src/provider/provider_tracking.h Outdated Show resolved Hide resolved

ldorau force-pushed the Use_base_allocator_in_the_tracking_provider branch from 480eaa7 to 226a268 Compare January 29, 2024 17:16

ldorau force-pushed the Use_base_allocator_in_the_tracking_provider branch from 226a268 to 9408546 Compare February 1, 2024 14:01

ldorau requested a review from igchor February 1, 2024 15:10

igchor reviewed Feb 1, 2024

View reviewed changes

igchor approved these changes Feb 1, 2024

View reviewed changes

ldorau force-pushed the Use_base_allocator_in_the_tracking_provider branch from 9408546 to 4fefff6 Compare February 1, 2024 21:24

igchor approved these changes Feb 1, 2024

View reviewed changes

bratpiorka requested changes Feb 2, 2024

View reviewed changes

src/provider/provider_tracking.c Show resolved Hide resolved

ldorau force-pushed the Use_base_allocator_in_the_tracking_provider branch from 4fefff6 to 96897cb Compare February 5, 2024 06:45

bratpiorka approved these changes Feb 5, 2024

View reviewed changes

ldorau added 2 commits February 5, 2024 09:55

Use base allocator in the tracking provider

004652b

Signed-off-by: Lukasz Dorau <[email protected]>

Remove util_mutex_create() and util_mutex_destroy()

faa65ee

Signed-off-by: Lukasz Dorau <[email protected]>

ldorau force-pushed the Use_base_allocator_in_the_tracking_provider branch from 96897cb to faa65ee Compare February 5, 2024 08:59

bratpiorka merged commit aff94a2 into oneapi-src:main Feb 5, 2024

ldorau deleted the Use_base_allocator_in_the_tracking_provider branch February 5, 2024 09:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Use base allocator in the tracking provider #170

Use base allocator in the tracking provider #170

Uh oh!

ldorau commented Jan 24, 2024 •

edited

Loading

Uh oh!

Uh oh!

igchor commented Jan 24, 2024

Uh oh!

Uh oh!

ldorau commented Jan 26, 2024

Uh oh!

Uh oh!

igchor commented Jan 29, 2024

Uh oh!

ldorau commented Jan 29, 2024

Uh oh!

ldorau commented Jan 29, 2024 •

edited

Loading

Uh oh!

igchor commented Jan 29, 2024

Uh oh!

ldorau commented Feb 1, 2024 •

edited

Loading

Uh oh!

igchor Feb 1, 2024

Uh oh!

ldorau Feb 1, 2024

Uh oh!

igchor commented Feb 1, 2024

Uh oh!

ldorau commented Feb 1, 2024

Uh oh!

ldorau commented Feb 1, 2024

Uh oh!

Uh oh!

Uh oh!

Use base allocator in the tracking provider #170

Use base allocator in the tracking provider #170

Uh oh!

Conversation

ldorau commented Jan 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

igchor commented Jan 24, 2024

Uh oh!

Uh oh!

ldorau commented Jan 26, 2024

Uh oh!

Uh oh!

igchor commented Jan 29, 2024

Uh oh!

ldorau commented Jan 29, 2024

Uh oh!

ldorau commented Jan 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

igchor commented Jan 29, 2024

Uh oh!

ldorau commented Feb 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

igchor Feb 1, 2024

Choose a reason for hiding this comment

Uh oh!

ldorau Feb 1, 2024

Choose a reason for hiding this comment

Uh oh!

igchor commented Feb 1, 2024

Uh oh!

ldorau commented Feb 1, 2024

Uh oh!

ldorau commented Feb 1, 2024

Uh oh!

Uh oh!

Uh oh!

ldorau commented Jan 24, 2024 •

edited

Loading

ldorau commented Jan 29, 2024 •

edited

Loading

ldorau commented Feb 1, 2024 •

edited

Loading