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

Link statically with custom jemalloc with disabled initial TLS #941

Conversation

ldorau
Copy link
Contributor

@ldorau ldorau commented Nov 27, 2024

Description

Link statically with custom jemalloc built from sources
with the following non-default options enabled:

  • --with-jemalloc-prefix=je_ - add je_ prefix to all public APIs
  • --disable-cxx - Disable C++ integration. This will cause new and delete operators implementations to be omitted.
  • --disable-initial-exec-tls - Disable the initial-exec TLS model for jemalloc's internal thread-local storage (on those platforms that support explicit settings). This can allow jemalloc to be dynamically loaded after program startup (e.g. using dlopen()).

Fixes: #891
Fixes: #894
Fixes: #903

Checklist

  • Code compiles without errors locally
  • All tests pass locally
  • CI workflows execute properly

@ldorau ldorau force-pushed the Link_statically_with_custom_jemalloc_with_disabled_initial_TLS branch 2 times, most recently from b9c4aff to 1478d88 Compare November 27, 2024 11:15
@ldorau ldorau force-pushed the Link_statically_with_custom_jemalloc_with_disabled_initial_TLS branch from 1478d88 to 48145aa Compare November 27, 2024 13:26
@ldorau ldorau requested review from bratpiorka and lplewa November 27, 2024 13:26
.github/workflows/reusable_multi_numa.yml Outdated Show resolved Hide resolved
.github/workflows/reusable_multi_numa.yml Outdated Show resolved Hide resolved
examples/dram_and_fsdax/CMakeLists.txt Outdated Show resolved Hide resolved
@ldorau ldorau force-pushed the Link_statically_with_custom_jemalloc_with_disabled_initial_TLS branch from 48145aa to b024e53 Compare November 27, 2024 14:04
@ldorau ldorau requested a review from bratpiorka November 27, 2024 14:05
@ldorau ldorau force-pushed the Link_statically_with_custom_jemalloc_with_disabled_initial_TLS branch 15 times, most recently from e437bbf to fd5bd4e Compare November 28, 2024 13:14
@ldorau ldorau marked this pull request as ready for review November 28, 2024 14:23
@ldorau ldorau requested a review from a team as a code owner November 28, 2024 14:23
@ldorau ldorau force-pushed the Link_statically_with_custom_jemalloc_with_disabled_initial_TLS branch from fd5bd4e to ee811cb Compare November 28, 2024 14:52
@ldorau
Copy link
Contributor Author

ldorau commented Nov 28, 2024

There is still one issue:

@ldorau
Copy link
Contributor Author

ldorau commented Nov 28, 2024

There is still one issue:

I am testing the previous release of jemalloc (v5.2.1) now...

@ldorau ldorau requested a review from KFilipek December 4, 2024 08:17
examples/CMakeLists.txt Outdated Show resolved Hide resolved
src/pool/CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@ldorau ldorau force-pushed the Link_statically_with_custom_jemalloc_with_disabled_initial_TLS branch from 5298b5e to aed8c12 Compare December 4, 2024 11:45
@ldorau ldorau requested review from PatKamin and vinser52 December 4, 2024 11:47
@ldorau ldorau force-pushed the Link_statically_with_custom_jemalloc_with_disabled_initial_TLS branch from aed8c12 to 1a11bb9 Compare December 4, 2024 12:37
Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>
Link statically with custom jemalloc built from sources
with the following non-default options enabled:

--with-jemalloc-prefix=je_ - add je_ prefix to all public APIs
--disable-cxx - Disable C++ integration. This will cause new and delete
                operators implementations to be omitted.
--disable-initial-exec-tls - Disable the initial-exec TLS model for jemalloc's
                internal thread-local storage (on those platforms that support
                explicit settings). This can allow jemalloc to be dynamically
                loaded after program startup (e.g. using dlopen).

Fixes: oneapi-src#891
Fixes: oneapi-src#894
Fixes: oneapi-src#903

Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>
@ldorau ldorau force-pushed the Link_statically_with_custom_jemalloc_with_disabled_initial_TLS branch from 1a11bb9 to fe7aaff Compare December 4, 2024 12:39
@ldorau ldorau requested a review from vinser52 December 4, 2024 12:42
@ldorau ldorau force-pushed the Link_statically_with_custom_jemalloc_with_disabled_initial_TLS branch 3 times, most recently from 63ecc30 to 2802736 Compare December 4, 2024 13:20
@@ -139,11 +139,13 @@ int main() {
// ctest looks for "PASSED" in the output
std::cout << "PASSED" << std::endl;

#if defined(UMF_BUILD_LIBUMF_POOL_DISJOINT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On L124 the following macro is used: #if defined(UMF_POOL_DISJOINT_ENABLED).

Please decide which is appropriate UMF_POOL_DISJOINT_ENABLED or UMF_BUILD_LIBUMF_POOL_DISJOINT and unify

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In our CMake files *_ENABLED = user set appropriate BUILD flag + all dependencies needed for build are satisfied so we actually can use given component.
@ldorau please use *_ENABLED in c/cpp files

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor

@vinser52 vinser52 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the remaining comment

README.md Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@ldorau ldorau force-pushed the Link_statically_with_custom_jemalloc_with_disabled_initial_TLS branch from 2802736 to cd4e7c2 Compare December 4, 2024 15:28
@ldorau ldorau force-pushed the Link_statically_with_custom_jemalloc_with_disabled_initial_TLS branch from cd4e7c2 to 003075f Compare December 4, 2024 15:42
Remove the separate static `jemalloc_pool` library.
Make the `UMF_BUILD_LIBUMF_POOL_JEMALLOC` option turned ON by default.
Incorporate jemalloc_pool into libumf.

Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>
@ldorau ldorau force-pushed the Link_statically_with_custom_jemalloc_with_disabled_initial_TLS branch from 003075f to 614c4b5 Compare December 4, 2024 16:01
benchmark/CMakeLists.txt Show resolved Hide resolved
@ldorau ldorau merged commit 27ee09f into oneapi-src:main Dec 5, 2024
77 checks passed
@ldorau ldorau deleted the Link_statically_with_custom_jemalloc_with_disabled_initial_TLS branch December 5, 2024 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants