Issue 7047 optional jemalloc

[amsmota]

PR description

Fixed Issue(s)

See #7047

Thanks for sending a pull request! Have you done the following?

• [x] Checked out our contribution guidelines?
• [ ] Considered documentation and added the doc-change-required label to this PR if updates are required.
• [ ] Considered the changelog and included an update if required.
• [ ] For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

• [ ] unit tests: ./gradlew build
• [ ] acceptance tests: ./gradlew acceptanceTest
• [ ] integration tests: ./gradlew integrationTest
• [ ] reference tests: ./gradlew ethereum:referenceTests:referenceTests

[fab-10]

@amsmota @usmansaleem I just had a different idea about how to implement, that is more in line with the requirements. Basically we have the besu-untuned script that seems to perfectly fit the purpose of having a vanilla Besu command, without any special options enforced, that can be used to perform any kind of customization, so I would rather suggest to keep besu command as is, and remove all the jemalloc related code from besu-untuned.

[amsmota]

@amsmota @usmansaleem I just had a different idea about how to implement, that is more in line with the requirements. Basically we have the besu-untuned script that seems to perfectly fit the purpose of having a vanilla Besu command, without any special options enforced, that can be used to perform any kind of customization, so I would rather suggest to keep besu command as is, and remove all the jemalloc related code from besu-untuned.

Hi, TBH I don't know what that besu-untuned is, do you have any link to docs or something? Anyway, besu already supports Jemalloc, right? They did intensive tests with several mallocs and they found out Jemalloc is the one that gives better performance, an it is part of the product now. Are they supposed to remove it from besu main? If not this PR fits the bill, it contains minimal changes to it and it works fine. So I would like to go ahead with this PR.

https://wiki.hyperledger.org/display/BESU/2024+-+Besu+Performance+Improvements+since+the+Merge https://wiki.hyperledger.org/display/BESU/Reduce+Memory+usage+by+choosing+a+different+low+level+allocator

On a personal note, I won't have much time to contribute to Besu since I moved from Blockchain to GenAI and I have all my time assigned to it, that's why I want to wrap up my 2 PRs as fast as I can...

Cheers.

[fab-10]

@amsmota besu-untuned is a start script, you can find it in the bin dir, that is like the besu standard script, but hasn't any preset configuration for Java memory, so you can use it to test your own setup, for example change GC, or other flags, that is exactly what you want to test a custom setup, to see if it is better than the proposed one. So removing the preset of the allocator from besu-untuned goes in the right directing of giving the freedom to the user to selected whichever allocator he want, and should have been already the case. Do not worry, I can pick this task if you haven't time now, and thanks for raising that requirement.

[amsmota]

@amsmota besu-untuned is a start script, you can find it in the bin dir

Oh, I saw it now... I thougth it was a fork of the main project or so... I never care to look at that folder after installDist... So with that "untuned" version it make all sense to remove any reference to Jemalloc as you say, and let it use the OS default Malloc and let the users/developers to do what they think it's better. However, the fact is that the main Besu already support that, for what I see since 22.7.0-RC2, so I think it's still good to provied this change to the "general" population that uses Besu out-of-the-box. In our case this concerns more the Prod deployment, because we need to quickly restart a deployment without Jemalloc if there are failures, and the current code will always load Jemalloc if it is available...

In the meantime I updated my branch to the latest main, I think I'll have to test everithing again...

Cheers.

[fab-10]

To use jemalloc with the besu-untuned script (when it will be changed), you just need to run

export LD_PRELOAD=libjemalloc.so; bin/besu-untuned

[amsmota]

Ok, guys, I updated and build my branch, I uninstalled jemalloc, tested with BESU_USING_JEMALLOC true and false and without it, all messages appeared correctly, I installed jemalloc again and tested with BESU_USING_JEMALLOC true and false and without it and all messages appeared correctly too.

I also tested removing all LD_PRELOAD from the scripts and you guys are right, it is needed to load jemalloc, my bad... It's not that Native.load won't load, it loads it but just for Java internal calls, but it does not influence the behavior of memory allocation.

I also tested with LD_PRELOAD but removing the Native.load part, it works fine but I have no way to confirm that jemalloc is loaded. However, I have no reason to believe it dosen't, so I assume it is loaded.

So what I am going to do is to remove the BESU_USING_JEMALLOC from the script, since it does not exist it will load jemalloc if exists any way, and let users set BESU_USING_JEMALLOC if and as they need it...

What do youse guys think?

[amsmota]

To use jemalloc with the besu-untuned script (when it will be changed), you just need to run

export LD_PRELOAD=libjemalloc.so; bin/besu-untuned

Yes, the only issue is that there will be nothing in the logs saying it is using it. But then again, whoever adds the LD_PRELOAD will know. It will be a nice-to-have to log some info, but will not change any behaviour...

[amsmota]

Guys, I really got this wrong, I assumed things that I shouldn't have assumed, like assuming that LD_PRELOAD and Native.load would both load the lib, which is true but they are used for diferent ends. Thank you guys for correcting me. However, my goal of be able to load or not load Jemmaloc when it does exist it's not achivable with any of the solutions I gave, so now what I think it should be done to achive that is change the unix script to be like

if [ -z "$BESU_USING_JEMALLOC" ] || { [ -n "$BESU_USING_JEMALLOC" ] && [ "$BESU_USING_JEMALLOC" != "FALSE" ] && [ "$BESU_USING_JEMALLOC" != "false" ]; }; then
    if [ "\$darwin" = "false" -a "\$msys" = "false" ]; then
      # check if jemalloc is available
      TEST_JEMALLOC=\$(LD_PRELOAD=libjemalloc.so sh -c true 2>&1)

      # if jemalloc is available the output is empty, otherwise the output has an error line
      if [ -z "\$TEST_JEMALLOC" ]; then
        export LD_PRELOAD=libjemalloc.so
      else
        # jemalloc not available, as fallback limit malloc to 2 arenas
        export MALLOC_ARENA_MAX=2
      fi
    fi
fi

or probably with that IF just before the export LD_PRELOAD.

I did this change but I'm not able to test it, for some starnge reason both in Win and WSL I'm getting this error that I never saw, never happened before, and googling gives me no idea. I tried the obvious, to delete the entire build dir and add 777 access to subdirs, but it keeps on giving that error.

• What went wrong: Execution failed for task ':evmToolStartScripts'.

Could not write to file 'C:\Users\anton\Development\OSPO\besu\build\scripts\evmtool'

Anyway, I'll commit in case any of youse are able to test... I'll be able to do some work/testing tomorrow and Monday, probably I'm going to delete my local and checkout again, and test properly. And I'll check your latest comment too.

Thank you guys for all the help.

Cheers.

[fab-10]

@amsmota use this code, since this is a template for the shell script, you need to escape $ and it is possible to simplify the test expression

if [ "\$darwin" = "false" -a "\$msys" = "false" ]; then
    if [ "\$BESU_USING_JEMALLOC" != "FALSE" -a "\$BESU_USING_JEMALLOC" != "false" ]; then
      # check if jemalloc is available
      TEST_JEMALLOC=\$(LD_PRELOAD=libjemalloc.so sh -c true 2>&1)

      # if jemalloc is available the output is empty, otherwise the output has an error line
      if [ -z "\$TEST_JEMALLOC" ]; then
        export LD_PRELOAD=libjemalloc.so
        export BESU_USING_JEMALLOC=true
      else
        # jemalloc not available, as fallback limit malloc to 2 arenas
        export MALLOC_ARENA_MAX=2
      fi
    fi
fi

[usmansaleem]

@amsmota See https://github.com/hyperledger/besu/pull/7424#issuecomment-2320674559 please.

[amsmota]

Hi @usmansaleem, @fab-10, I finished today my other PR so I'll be looking at this one. If you have further comments besides the ones above please let me know.

Cheers.

[amsmota]

@amsmota See #7424 (comment) please.

Hi @fab-10, I saw it now, I'm sorry for the delay but I've been busy at work and also with my other PR. It looks good to me, yes. I'll try to test in my local and let youse know.

Cheers.

[amsmota]

Guys, again I'm sorry for the delays, I did changed my code with some changes requested but I have issues with my Windows 11 / WSL, specifically with Gradle in WSL. I may opt to push the code as it is and ask youse reviewers to test yourselves if I can’t fix that WSL.

Cheers.

[amsmota]

@usmansaleem, @fab-10, I deleted everithing from my WSL, both the code and the ~/.gradle. I then run a ./gradlew build but it fails:

Task :evmToolStartScripts FAILED

FAILURE: Build failed with an exception.

• What went wrong: Execution failed for task ':evmToolStartScripts'.

Could not write to file '/home/amsmota/Development/besu/build/scripts/evmtool'.

I am running with my user user, not with sudo, maybe I should try to do it as su?

Anyway, did youse saw this issue before and/or know how to fix it?

Cheers.

[amsmota]

Could not write to file '/home/amsmota/Development/besu/build/scripts/evmtool'.

Well, I fixed this one 👍 , which is good because it was I that caused...... 👎. And BTW the fix was actually the @fab-10 code from https://github.com/hyperledger/besu/pull/7424#issuecomment-2320674559. I could swear I had done that at some point but it seems I didn't...

But anyway, I have another error, one that I had before when working in https://github.com/hyperledger/besu/pull/6965 and I couldn't solve, and it is is a linux error too...

FAILURE: Build failed with an exception.

• What went wrong: Gradle build daemon disappeared unexpectedly (it may have been killed or may have crashed)

This doesen't happen building on windows, only in WSL.

There too some lines that I don't know of are related, I'll try to investigate that.

Java HotSpot(TM) 64-Bit Server VM warning: Sharing is only supported for boot loader classes because bootstrap classpath has been appended

Cheers.

[amsmota]

@fab-10 , @usmansaleem I finnally was able to build my branch and tested it and IT'S ALL GOOD !!!!

This are the tests I did and the results output in the log:

NO Jemalloc Installed / NO BESU_USING_JEMALLOC jemalloc library not found, memory usage may be reduced by installing it

NO Jemalloc Installed / BESU_USING_JEMALLOC = false BESU_USING_JEMALLOC is present but is not set to true, jemalloc library not loaded

NO Jemalloc Installed / BESU_USING_JEMALLOC = true BESU_USING_JEMALLOC is present but we failed to load jemalloc library to get the version

WITH Jemalloc Installed / NO BESU_USING_JEMALLOC jemalloc: 5.2.1-0-gea6b3e973b477b8061e0076bb257dbd7f3faa756

WITH Jemalloc Installed / BESU_USING_JEMALLOC = false BESU_USING_JEMALLOC is present but is not set to true, jemalloc library not loaded

WITH Jemalloc Installed / BESU_USING_JEMALLOC = true jemalloc: 5.2.1-0-gea6b3e973b477b8061e0076bb257dbd7f3faa756

I still have some work to do, namelly

• fix some jemalloc tests that are now failing
• change those outputs above to print BESU_USING_JEMALLOC in lowers (I don't like the way they look in capitals in the log) "besu_using_jemalloc is present but we failed to load jemalloc library to get the version"
• make a few changes in the script

A little concern I had is that I installed jemalloc on Ubuntu using the libjemalloc2 lib, that installs it as libjemalloc.so.2 . I had to rename it libjemalloc.so to make it compatible with the script, a note somewhere should warn about this...

Anyhow, by tomorrow or Saturday I should have this pushed to the PR.

Cheers.

[amsmota]

@fab-10 , @usmansaleem et all, I finally finsihed my changes, the failing test is now running and the other small changes done too. I tested again on my WSL and it all seems right.

Can youse please review the changes and hopefully test and give me some feedback? Please note that the expected scenarios that I tested are:

1. NO Jemalloc Installed / NO BESU_USING_JEMALLOC
2. NO Jemalloc Installed / BESU_USING_JEMALLOC = false
3. NO Jemalloc Installed / BESU_USING_JEMALLOC = true
4. WITH Jemalloc Installed / NO BESU_USING_JEMALLOC
5. WITH Jemalloc Installed / BESU_USING_JEMALLOC = false
6. WITH Jemalloc Installed / BESU_USING_JEMALLOC = true

and the results in the logs shloud be

1. jemalloc library not found, memory usage may be reduced by installing it
2. besu_using_jemalloc is present but is not set to true, jemalloc library not loaded
3. besu_using_jemalloc is present but we failed to load jemalloc library to get the version
4. jemalloc: 5.2.1-0-gea6b3e973b477b8061e0076bb257dbd7f3faa756
5. besu_using_jemalloc is present but is not set to true, jemalloc library not loaded
6. jemalloc: 5.2.1-0-gea6b3e973b477b8061e0076bb257dbd7f3faa75

(jemalloc version can be diferent, of course)

Guys, I can't thank you enough for your help, If you ever come to Dublin (In Ireland, not Ohio, believe me or not Facebook already tagged me in Ohio a few times!!!) I will gladly offer youse a couple of Guinnessess (it's a non written rule here that youse shall not drink only one)....

Cheers.

[amsmota]

Hi @fab-10 , I see you aproved my last changes, can you tell me what are the next steps for me to be able to close this PR?

@usmansaleem I see one requested change made by you, can you approve/dismiss it?

Thank you guiys.

[usmansaleem]

@amsmota You accidently closed it, I've reopened it and enabled automerge. What you need to do is merge the main branch back onto your branch. Once you have up-to-date changes from main branch, due to automerge enabled, it will merge once all tests pass.

[amsmota]

Hi @usmansaleem, thanks for your quick reply. I just did that merge so I'll wait for the tests to be run and close the PR.

Thank you again for you colaboration, it is greatly appreciated.

Cheers.

[usmansaleem]

@amsmota you don't need to "close" PR, once PR is merged to main, its status will automatically changed.

[amsmota]

@amsmota you don't need to "close" PR, once PR is merged to main, its status will automatically changed.

So I can close the PR now?

[usmansaleem]

@amsmota As I said, if you close the PR it won't merge. As we have enabled "auto merge", it will merge on its own. Just sit back and relax :)

[amsmota]

@amsmota As I said, if you close the PR it won't merge. As we have enabled "auto merge", it will merge on its own. Just sit back and relax :)

Got it 👍

[amsmota]

Guys, @usmansaleem and @fab-10 and everybody that helped with sugestions and comments my sincere gratitude, it was a task way more dificult than I initially supposed and I wouldn't be sucesffull in doing it without yours support. It seems that saying "standing in the shoulder of giants" it's actually true...

Well, "may the road rise to meet youse" as they say here in Ireland... 💯