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

Update jemalloc to version 5.3.0 #1337

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fel1x-developer
Copy link
Contributor

@fel1x-developer fel1x-developer commented Jul 22, 2024

Move to Github PR by @bsdimp 's request through freebsd-current mailiing list.

Phabricator review

jemalloc 5.3.0 was released in 2022 but has not been imported into FreeBSD base system yet. The new version of jemalloc includes new features and bug fixes which are listed on release note.

@brooksdavis
Copy link
Contributor

This will need an update post 5680cf6

@fel1x-developer
Copy link
Contributor Author

git rebase main

@bsdimp
Copy link
Member

bsdimp commented Aug 23, 2024

We'll need an exp run and this needs to be redone as a vendor branch update (which I'm on the hook for).....

@bsdimp bsdimp self-assigned this Aug 23, 2024
@fel1x-developer
Copy link
Contributor Author

@bsdimp I don't have src commit bit which is required for vendor import. Can you import jemalloc 5.3.0 in vendor/jemalloc for me please? I will take the rest of work (merging, editing Makefile...)

@bsdimp
Copy link
Member

bsdimp commented Sep 22, 2024

I'll import the 5.3.0 into vendor/jemalloc, but it appears that doesn't exist yet, so I'll need to look closely at what needs to happen to make it happen. Looks like it's never really been imported via the svn vendor tree. I need to go study this more closely to know the right thing to import. Sorry this is taking so long

@lwhsu
Copy link
Member

lwhsu commented Sep 22, 2024

Yeah sorry that I was also occupied by other things. I got some issues when testing this on arm64 (not related to jemalloc but arm64 itself). I'll work on test this in amd64 and arm64.

@fel1x-developer
Copy link
Contributor Author

Reflected upstream changes by rebasing main. 5.3.0 vendor import is required.

@bsdimp
Copy link
Member

bsdimp commented Dec 10, 2024

Thanks for the rebase.
How'd you generate things? I have a rather large diff to your stuff in the sanity check diff I did.
And how'd you work around the _pthread_getname_np issue? I think it's a namespace issue.
I have lots of questions...
I'm also having a build issue with _pthread_getaffinity_np, but I think that's because autogen / ./configure didn't set JEMALLOC_HAVE_SECURE_GETENV, JEMALLOC_HAVE_PTHREAD_SETNAME_NP, JEMALLOC_HAVE_SCHED_GETCPU or JEMALLOC_HAVE_SCHED_SETAFFINITY. Were those enabled by hand or ???

So far I've done this by checking out the 5.3.0 tag in a git tree, copying all the files over to my vendor import tree, then running

% autogen
% ./configure --enable-autoconf --with-version=5.3.0-0-g54eaed1d8b56b1aa528be3bdd1877e59c56fa90c --with-xslroot=/usr/local/share/xsl/docbook
% gmake build_doc

and copying everything over.

But, it looks like the various .sh -> .h scripts didn't get run quite right.

I'm seeing issues with the fact that pthread_np.h is included inside a 'namespace.h' block, but we unnamespace before we build jemalloc.c (still looking into how we do that).

I do see that setting JEMALLOC_HAVE_SCHED_* defines in my tree lets it compile.

So understanding how you got here is going to be critical if we ever want to update it again in the future.

@fel1x-developer
Copy link
Contributor Author

@bsdimp I used FREEBSD-upgrade script under contrib/jemalloc. I'll try the merge again and see if there is any problem.

@bsdimp
Copy link
Member

bsdimp commented Dec 11, 2024

@bsdimp I used FREEBSD-upgrade script under contrib/jemalloc. I'll try the merge again and see if there is any problem.

Where did you get the tree you used for the upstream part of that? Oh, I didn't see you'd updated it... I need to study it a bit... But it does look like it will answer many of my questions I think... There's stuff I still don't quite understand, but I'd like to get to the bottom of it all so that I can get the right stuff into the 'update the vendor branch' stuff since that's typically how we've updated things since the migration to git. It's also providing a much higher level of confidence for me that there's nothing rogue in this update (not that I suspect you personally, but after xz I have to make sure that my trust isn't misplaced since a tiny number of people spoil the fun for everyone else).

@fel1x-developer
Copy link
Contributor Author

And how'd you work around the _pthread_getname_np issue? I think it's a namespace issue.

https://reviews.freebsd.org/D41461 might be related to this issue.

@fel1x-developer fel1x-developer force-pushed the jemalloc branch 2 times, most recently from ef5b3b6 to ce5555a Compare December 14, 2024 02:18
Summary: jemalloc 5.3.0 was released in 2022 but has not been imported into FreeBSD base system yet. The new version of jemalloc includes new features and bug fixes which are listed on [[ https://github.com/jemalloc/jemalloc/releases/tag/5.3.0 | release note ]].

Test Plan:
```
cd lib/libc
make
cd /usr/src/
make buildworld && make installworld
```
then reboot and test several programs and applications

Reviewers: #contributor_reviews_src, vangyzen

Subscribers: jasone, vangyzen, instructionset_gmail.com, jrtc27, brooks, gbe, emaste, lwhsu, #contributor_reviews_src, imp

Tags: #contributor_reviews_src

Differential Revision: https://reviews.freebsd.org/D41421
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants