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

jemalloc_pool should not require user to link with jemalloc #891

Closed
lplewa opened this issue Nov 12, 2024 · 15 comments · Fixed by #941
Closed

jemalloc_pool should not require user to link with jemalloc #891

lplewa opened this issue Nov 12, 2024 · 15 comments · Fixed by #941
Assignees
Labels
bug Something isn't working

Comments

@lplewa
Copy link
Contributor

lplewa commented Nov 12, 2024

Currently user needs to pass -ljemalloc linker flag if uses jemalloc pool (jemalloc_pool.a)

This is an issue as jemalloc exports malloc, realloc, free symbols, so actually liking with jemalloc overrides system functions.
That is annoying for the user, who don't want to use jmalloc instead of glibc functions.

This is also a problem for proxy lib's threshold #883 as dlsym(RTLD_NEXT, "malloc"); will return malloc@jemalloc not malloc@glibc. See also #894.

to overcome this issue we can:

  • use dlopen, and dlsym in jemalloc_pool
  • link statically with jemaloc in jemalloc_pool

Not sure what is a better approach.

@lplewa lplewa added the bug Something isn't working label Nov 12, 2024
@vinser52
Copy link
Contributor

Just to note here what I mentioned on the UMF Technical meeting:

link statically with jemalloc in jemalloc_pool

It would mean that UMF is hardcoded to use a particular version of jemalloc, In the past we heard a lot of concerns from Memkind customers/questions regarding how to force Memkind to use different versions of jemalloc. With the current design of UMF users can just upgrade jemalloc to the required version. But if we link statically then UMF should be rebuilt to use a different version of jemalloc.

@bratpiorka
Copy link
Contributor

I'm for dynamic linking

@lplewa
Copy link
Contributor Author

lplewa commented Nov 13, 2024

statically link allows use to use CTL macros from jemaloc header - but by looking on the header, there is not a huge amount of macros which looks useful for us, so we should be fine.

@pbalcer can to show any examples of jemalloc macros which might be a limiting factor for us if we decide to dynamic link jemaloc with umf.

@lplewa
Copy link
Contributor Author

lplewa commented Nov 18, 2024

I talked with Piotr, and i convinced him that we can go with dynamic link to fix this issue

@ldorau ldorau self-assigned this Nov 19, 2024
@ldorau
Copy link
Contributor

ldorau commented Nov 20, 2024

@bratpiorka @vinser52 @lplewa @pbalcer there is an issue with dlopen(libjemalloc.so):

Dynamically loading the jemalloc library (using dlopen(libjemalloc.so)) requires rebuilding it from sources with the "--disable-initial-exec-tls" option, because of the following issue: jemalloc/jemalloc#1237

I get the following error now:

[FATAL UMF] utils_open_library: dlopen(libjemalloc.so.2) failed with error: /usr/lib/x86_64-linux-gnu/libjemalloc.so.2: cannot allocate memory in static TLS block

See: #919

ldorau added a commit to ldorau/unified-memory-framework that referenced this issue Nov 20, 2024
Do not link with the jemalloc library explicitly,
but load it dynamically (using dlopen()).

Ref: oneapi-src#891
Ref: oneapi-src#894

Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>
@bratpiorka
Copy link
Contributor

@vinser52 @lplewa @pbalcer what do you think of the following combo:
when jemalloc is required: fetch jemalloc from github + build with required flags + add an option to use jemalloc the user-provided path (this is a user's responsibility to build it properly) + load so dynamically + ship .so with UMF for oneAPI package

@lplewa
Copy link
Contributor Author

lplewa commented Nov 20, 2024

@vinser52 @lplewa @pbalcer what do you think of the following combo: when jemalloc is required: fetch jemalloc from github + build with required flags + add an option to use jemalloc the user-provided path (this is a user's responsibility to build it properly) + load so dynamically + ship .so with UMF for oneAPI package

Fetch jemalloc with cmake option to use different version, and link statically. You approach have an issue that on make install of umf we have to install our version of jemalloc.so, what is not a great idea.

ldorau added a commit to ldorau/unified-memory-framework that referenced this issue Nov 21, 2024
Do not link with the jemalloc library explicitly,
but load it dynamically (using dlopen()).

Ref: oneapi-src#891
Ref: oneapi-src#894

Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>
@pbalcer
Copy link
Contributor

pbalcer commented Nov 21, 2024

The main argument for using dlopen was that it'd be possible for users to supplement their own jemalloc from the OS. That is no longer the case, and would require us to explain to users that they need to have a specially built jemalloc with the tls flag.
At this point I think the cons outweigh the pros of using dlopen. I suggest we just do what memkind does and build/link with jemalloc statically (setting --disable-initial-exec-tls to fix any potential issues with dlopening umf itself).

@vinser52
Copy link
Contributor

The main reason to link dynamically was the following: we remember that Memkind users asked from time to time that they would like to use different version of jemalloc with Memkind. If we cannot reach it we UMF than we can just link statically with particular version of jemalloc.

Regarding dlopen approach, what if we will create a proxy lib ( e.g. jemalloc_proxy, name could be different :) ) that will have a hard dependency to jemalloc.so. umf will try to dlopen the jemalloc_proxy. Since jemalloc_proxy depends on libjemalloc.so the dlopen will successes if libjemalloc.so is available on the system or fail otherwise.

@vinser52
Copy link
Contributor

The main argument for using dlopen was that it'd be possible for users to supplement their own jemalloc from the OS. That is no longer the case, and would require us to explain to users that they need to have a specially built jemalloc with the tls flag. At this point I think the cons outweigh the pros of using dlopen. I suggest we just do what memkind does and build/link with jemalloc statically (setting --disable-initial-exec-tls to fix any potential issues with dlopening umf itself).

@pbalcer explains almost the same :). I agree that if we are not able to allow users to choose the version of jemalloc than we should just link statically. But we can try the idea that I suggested in my previous comment.

@bratpiorka
Copy link
Contributor

I would still like to provide the user with a way to rebuild the UMF with the jemalloc sources specified by the user. Is this possible?

@lplewa
Copy link
Contributor Author

lplewa commented Nov 21, 2024

I would still like to provide the user with a way to rebuild the UMF with the jemalloc sources specified by the user. Is this possible?

Yeah - we will have a GH link with commit hash - in our cmake - user can just change. We can make also cmake variable to override it, so there is no need to change our files, but i prefer to not do so - we can add it always latter on, if someone will ask us. (we have tooo many variables in our cmake, so we should try to not add new if it is not needed)

@vinser52
Copy link
Contributor

I would still like to provide the user with a way to rebuild the UMF with the jemalloc sources specified by the user. Is this possible?

Yeah - we will have a GH link with commit hash - in our cmake - user can just change. We can make also cmake variable to override it, so there is no need to change our files, but i prefer to not do so - we can add it always latter on, if someone will ask us. (we have tooo many variables in our cmake, so we should try to not add new if it is not needed)

I think @bratpiorka wanted an ability to load jemalloc dynamically if the user builds it from the source with --disable-initial-exec-tls option.

@pbalcer
Copy link
Contributor

pbalcer commented Nov 21, 2024

Regarding dlopen approach, what if we will create a proxy lib ( e.g. jemalloc_proxy, name could be different :) ) that will have a hard dependency to jemalloc.so. umf will try to dlopen the jemalloc_proxy. Since jemalloc_proxy depends on libjemalloc.so the dlopen will successes if libjemalloc.so is available on the system or fail otherwise.

That unfortunately won't work. The problem is transitive, dlopen would now complain that the jemalloc_proxy library is using too much TLS.

ldorau added a commit to ldorau/unified-memory-framework that referenced this issue Nov 27, 2024
Link statically with custom jemalloc built from sources
with the following options enabled:
--with-jemalloc-prefix=je_
--disable-initial-exec-tls

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

Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>
ldorau added a commit to ldorau/unified-memory-framework that referenced this issue Nov 27, 2024
Link statically with custom jemalloc built from sources
with the following options enabled:
--with-jemalloc-prefix=je_
--disable-initial-exec-tls

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

Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>
ldorau added a commit to ldorau/unified-memory-framework that referenced this issue Nov 27, 2024
Link statically with custom jemalloc built from sources
with the following options enabled:
--with-jemalloc-prefix=je_
--disable-initial-exec-tls

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

Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>
ldorau added a commit to ldorau/unified-memory-framework that referenced this issue Nov 27, 2024
Link statically with custom jemalloc built from sources
with the following options enabled:
--with-jemalloc-prefix=je_
--disable-initial-exec-tls

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

Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>
ldorau added a commit to ldorau/unified-memory-framework that referenced this issue Nov 28, 2024
Link statically with custom jemalloc built from sources
with the following options enabled:
--with-jemalloc-prefix=je_
--disable-initial-exec-tls

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

Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>
ldorau added a commit to ldorau/unified-memory-framework that referenced this issue Nov 28, 2024
Link statically with custom jemalloc built from sources
with the following options enabled:
--with-jemalloc-prefix=je_
--disable-initial-exec-tls

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

Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>
ldorau added a commit to ldorau/unified-memory-framework that referenced this issue Nov 28, 2024
Link statically with custom jemalloc built from sources
with the following options enabled:
--with-jemalloc-prefix=je_
--disable-initial-exec-tls

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

Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>
ldorau added a commit to ldorau/unified-memory-framework that referenced this issue Nov 28, 2024
Link statically with custom jemalloc built from sources
with the following options enabled:
--with-jemalloc-prefix=je_
--disable-initial-exec-tls

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

Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>
ldorau added a commit to ldorau/unified-memory-framework that referenced this issue Nov 28, 2024
Link statically with custom jemalloc built from sources
with the following options enabled:
--with-jemalloc-prefix=je_
--disable-initial-exec-tls

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

Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>
ldorau added a commit to ldorau/unified-memory-framework that referenced this issue Nov 28, 2024
Link statically with custom jemalloc built from sources
with the following options enabled:
--with-jemalloc-prefix=je_
--disable-initial-exec-tls

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

Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>
ldorau added a commit to ldorau/unified-memory-framework that referenced this issue Nov 28, 2024
Link statically with custom jemalloc built from sources
with the following options enabled:
--with-jemalloc-prefix=je_
--disable-initial-exec-tls

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

Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>
ldorau added a commit to ldorau/unified-memory-framework that referenced this issue Nov 29, 2024
Link statically with custom jemalloc built from sources
with the following options enabled:
--with-jemalloc-prefix=je_
--disable-initial-exec-tls

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

Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>
ldorau added a commit to ldorau/unified-memory-framework that referenced this issue Nov 29, 2024
Link statically with custom jemalloc built from sources
with the following options enabled:
--with-jemalloc-prefix=je_
--disable-initial-exec-tls

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

Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>
ldorau added a commit to ldorau/unified-memory-framework that referenced this issue Dec 2, 2024
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
Copy link
Contributor

ldorau commented Dec 2, 2024

The fix is ready, please review: #941

ldorau added a commit to ldorau/unified-memory-framework that referenced this issue Dec 3, 2024
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 added a commit to ldorau/unified-memory-framework that referenced this issue Dec 4, 2024
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 added a commit to ldorau/unified-memory-framework that referenced this issue Dec 4, 2024
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 closed this as completed in e95d92e Dec 5, 2024
lplewa pushed a commit to lplewa/unified-memory-framework that referenced this issue Dec 10, 2024
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants