Skip to content

Jemalloc : Enable tls_model("initial-exec")#263

Merged
johnhaddon merged 1 commit intoGafferHQ:8_maintenancefrom
murraystevenson:jemallocTLS
May 14, 2024
Merged

Jemalloc : Enable tls_model("initial-exec")#263
johnhaddon merged 1 commit intoGafferHQ:8_maintenancefrom
murraystevenson:jemallocTLS

Conversation

@murraystevenson
Copy link
Contributor

This avoids the potential of infinite recursion with dynamic TLS on modern glibc versions, as noted here: https://lists.gnu.org/archive/html/info-gnu/2024-01/msg00017.html

  The dynamic linker calls the malloc and free functions in more cases
  during TLS access if a shared object with dynamic TLS is loaded and
  unloaded.  This can result in an infinite recursion if a malloc
  replacement library or its dependencies use dynamic TLS instead of
  initial-exec TLS.

I'm not overly clear on the broader ramifications of this change without wider testing, but I've noticed other DCCs that ship Jemalloc 3.6.0 recently providing the same tls-model change for glibc 2.39 compatibility.

The patch is based on the below commit for Jemalloc 4.0.0, though configure is being patched instead of configure.ac as we call configure directly.

jemalloc/jemalloc@f1cf3ea

This suggests that initial-exec was the intended default but the test has been long broken...

This avoids the potential of infinite recursion with dynamic TLS on modern glibc versions, as noted here: https://lists.gnu.org/archive/html/info-gnu/2024-01/msg00017.html

```
  The dynamic linker calls the malloc and free functions in more cases
  during TLS access if a shared object with dynamic TLS is loaded and
  unloaded.  This can result in an infinite recursion if a malloc
  replacement library or its dependencies use dynamic TLS instead of
  initial-exec TLS.
```

The patch is based on the below commit for Jemalloc 4.0.0, though `configure` is being patched instead of `configure.ac` as we call `configure` directly.

jemalloc/jemalloc@f1cf3ea

This suggests that `initial-exec` was the intended default but the test has been long broken.
@murraystevenson murraystevenson self-assigned this May 13, 2024
@johnhaddon
Copy link
Member

LGTM. A bit of reading suggests that initial-exec would be the preferred TLS mode from a performance perspective, and as you say, the commit you linked makes it clear that it was also the intended model in jemalloc all along.

I did some limited performance testing to corroborate this, with gaffer stats and a SceneReader loading a large USD model. All results are best of three runs, normalized against Vanilla 1.4 numbers :

Runtime Max Resident Size
Vanilla 1.4 1.00 1.0
GAFFER_JEMALLOC=0 1.08 0.98
initial-exec 0.99 1.0
Jemalloc 5.3.0 0.97 1.02

That corroborates that this patch is a good thing, that Jemalloc is still a win over default malloc, and also suggests that Jemalloc 5.3.0 might be worth considering in future, with broader testing.

@johnhaddon johnhaddon merged commit d78bf8f into GafferHQ:8_maintenance May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants