[SERVER-51722] Ensure that MongoDB builds with ARM LSE atomics Created: 19/Oct/20 Updated: 29/Oct/23 Resolved: 23/Feb/21 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 4.9.0, 4.2.13, 4.4.5 |
| Type: | Task | Priority: | Major - P3 |
| Reporter: | Alex Cameron (Inactive) | Assignee: | Andrew Morrow (Inactive) |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||
| Backwards Compatibility: | Fully Compatible | ||||||||||||||||||||
| Backport Requested: |
v4.4, v4.2
|
||||||||||||||||||||
| Sprint: | Dev Platform 2021-03-08 | ||||||||||||||||||||
| Participants: | |||||||||||||||||||||
| Description |
|
WiredTiger recently got an external contribution from Github user AGSaidi to use the -moutline-atomics flag for ARM in the cases where the compiler supports it. In GCC-10, this flag is on by default. The functionality has apparently been backported to older versions of GCC however it needs to be explicitly set with the -moutline-atomics flag. On my local machine, I saw wtperf ops/sec increase by 2x on a few of the workloads that I tested. If you're already aware of this flag, feel free to close this. I figured I should let someone from the server org know just in case. |
| Comments |
| Comment by Andrew Morrow (Inactive) [ 20/Jul/21 ] | |
|
tsahee@amazon.com - FYI, the v4.4 release with -moutline-atomics in effect seems to be bringing significant performance improvements for at least one user: see https://jira.mongodb.org/browse/SERVER-56237?page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel&focusedCommentId=3952147#comment-3952147. Just thought you would like to know. Thanks for your help and guidance as we got it rolled out. | |
| Comment by Tsahi Zidenberg [ 09/Mar/21 ] | |
|
@andrew.morrow Assuming these are different locks - generally speaking there is no complicated inter-dependency. Locks using LSE will enjoy LSE performance, locks using exclusive access will see exclusive-access performance, regardless of how many of each you have. | |
| Comment by Andrew Morrow (Inactive) [ 09/Mar/21 ] | |
|
Hi tsahee@amazon.com - A question for you on this ticket. billy.donahue noticed that our vendored boost implementation does not know to use LSE instructions when available: see https://github.com/mongodb/mongo/blob/master/src/third_party/boost-1.70.0/boost/atomic/detail/extra_ops_gcc_arm.hpp. We believe that newer boost addresses this (see https://github.com/boostorg/atomic/commits/develop/include/boost/atomic/detail/extra_ops_gcc_aarch64.hpp), however, we don't tend to upgrade boost on stable branches unless absolutely necessary, so this is unlikely to change for v4.4 and older. We were curious if there were any adverse consequences to having both the LSE instructions and non-LSE instruction sequences executing in the same process. In other words, if we didn't upgrade boost, would the presence of the non-LSE instructions that it would emit somehow undermine the correctness or potential performance benefits of using the LSE instructions achieved via -moutline-atomics, such that we should consider making the upgrade? It seems unlikely, but we thought it would be good to double check. | |
| Comment by Githook User [ 23/Feb/21 ] | |
|
Author: {'name': 'Andrew Morrow', 'email': 'acm@mongodb.com', 'username': 'acmorrow'}Message: (cherry picked from commit 63a5a847c4ef2864e27cd406812bf47921d16e73) | |
| Comment by Githook User [ 23/Feb/21 ] | |
|
Author: {'name': 'Andrew Morrow', 'email': 'acm@mongodb.com', 'username': 'acmorrow'}Message: (cherry picked from commit 63a5a847c4ef2864e27cd406812bf47921d16e73) | |
| Comment by Andrew Morrow (Inactive) [ 23/Feb/21 ] | |
|
The new flag is added if supported by the compiler. Note that this won't affect our binary releases until we upgrade our toolchain to GCC 8.5, but it will mean that for others building with compilers that offer support for the flag, they will get the improved behavior automatically. | |
| Comment by Githook User [ 22/Feb/21 ] | |
|
Author: {'name': 'Andrew Morrow', 'email': 'acm@mongodb.com', 'username': 'acmorrow'}Message: | |
| Comment by Andrew Morrow (Inactive) [ 04/Feb/21 ] | |
|
We should backport this through to the oldest branch that will be affected when we bump our toolchain to GCC 8.5. | |
| Comment by Andrew Morrow (Inactive) [ 27/Jan/21 ] | |
|
Thanks tsahee@amazon.com, we will give those a look. | |
| Comment by Tsahi Zidenberg [ 26/Jan/21 ] | |
|
This is the list of commits (in order) to have -moutline-atomics: https://gcc.gnu.org/git?p=gcc.git;a=commitdiff;h=512b0ffab3bc4f334cbb36c598192c1da2abe330
That's more commits than I expected, but on the bright side - there's a good chance they will apply cleanly on top of 8.3. | |
| Comment by Andrew Morrow (Inactive) [ 26/Jan/21 ] | |
|
We are pretty conservative about toolchain upgrades, so I think I'd have a hard time justifying upgrading to gcc-8 HEAD. If the list of commits is reasonably small we can give them a look and make a call about how comfortable we are with pulling them in and whether they apply without issue. Otherwise, it isn't that long a wait until end of March, in the grand scheme of things. | |
| Comment by Tsahi Zidenberg [ 26/Jan/21 ] | |
|
@Andrew Morrow Talked with Sebastian Pop. He told me:
| |
| Comment by Andrew Morrow (Inactive) [ 26/Jan/21 ] | |
|
tsahee@amazon.com - Yes, we could look at cherry-picking it into our GCC build (which is currently at GCC 8.3). Do you happen to have a link to the GCC 8.5 commit handy? If not, no worries, I can dig it up. I can see if it applies cleanly for us so that we can get this done without waiting for GCC 8.5. | |
| Comment by Tsahi Zidenberg [ 26/Jan/21 ] | |
|
Hi Andrew! Gcc version releases tend to be about one year apart. 8.2 was released on July, 2017. 8.3 was released February, 2019 and 8.4 on March 2020. I am not closely involved with the project. My understanding is that it is expected in a similar schedule. I would guess around March April 2021, but your guess is as good as mine. It will be definitely be released, and it is expected to be the last official gcc 8.x release. GCC has a pretty slow release cycle, which is why some distributions, notably Ubuntu 20.04 and Amazon Linux 2 cherry-picked "-moutline atomics" and released it early. | |
| Comment by Andrew Morrow (Inactive) [ 22/Jan/21 ] | |
|
Hi tsahee@amazon.com - Do you happen to know if a GCC 8.5 release which would contain support for that flag is actually coming? The most recent information I can find on it is here https://gcc.gnu.org/legacy-ml/gcc/2020-03/msg00043.html, but that is from nearly a year ago. It sounds like you have been closely involved with getting this moved into the various compiler versions, so I'm hoping you might have some insight, or be able to help urge such a release to happen. | |
| Comment by Andrew Morrow (Inactive) [ 19/Jan/21 ] | |
|
tsahee@amazon.com - Thanks. It looks like that flag only appears in GCC 10's documentation, and I was searching in GCC 8 since that is currently our toolchain of record. When GCC 8.5 is released we will see about upgrading to it and including this flag in our builds. | |
| Comment by Tsahi Zidenberg [ 19/Jan/21 ] | |
|
Reference about ignoring the flag in newer architectures: About versions that support -moutline-atomics flag: | |
| Comment by Andrew Morrow (Inactive) [ 19/Jan/21 ] | |
|
For future reference, here is some good background on this: https://github.com/aws/aws-graviton-getting-started/blob/master/c-c%2B%2B.md | |
| Comment by Andrew Morrow (Inactive) [ 19/Jan/21 ] | |
|
tsahee@amazon.com - Thanks for your note. That is actually useful information, because I was somewhat reluctant to add this flag since it costs an extra load and branch on platforms where the LSE support is known present. However, I'm unable to find any documentation of this flag at all in GCCs documentation, which is a little puzzling. Do you have a reference that confirms that -moutline-atomics is ignored when targeting armv8.1a or newer with -march? Do you have a reference which indicates which GCC versions support the flag? Meanwhile, we should probably introduce armv8.2-a specific builds so we can capitalize on LSE without the extra load and branch. | |
| Comment by Tsahi Zidenberg [ 17/Jan/21 ] | |
|
When building for armv8.1-a, -moutline-atomic is ignored by GCC. If mongodb is built for arm8.1 by default this flag is indeed unnecessary. However, I understand it is not the current default arm64 architecture. Recommended mongodb installation seems to be from binaries. Distributing binaries built for arm v8.0 without outline-atomics could seriously performance on ARM for the reasonable user. | |
| Comment by Andrew Morrow (Inactive) [ 04/Jan/21 ] | |
|
It is a little unclear to me if -moutline-atomics would continue to be the right thing if we later changed our target architecture minimum to ARMv8.1a. Given that, I would prefer Ryan's approach of handling it outside the build system via additional CCFLAGS. Note that this is already how we select the target architecture (i.e. -march=armv8a+crc) in the evergreen yaml. | |
| Comment by Ryan Egesdahl (Inactive) [ 13/Nov/20 ] | |
|
alex.cameron we can, sort of, but it would take a bit more work to make our particular configuration able to do that where we need it to be. Either way, I would much rather keep track of this as-is so we know when the flag is actually useful with our toolchain, one way or another, so we also know when we can expect this change to be beneficial. For builds that do not use the MongoDB toolchain, you can always pass them via the CCFLAGS variable on the SCons command line, like so:
| |
| Comment by Alex Cameron (Inactive) [ 12/Nov/20 ] | |
|
Hey ryan.egesdahl, not sure if you can do this with SCons, but our AutoTools configuration just applies the flag on ARM whenever it's available. So our build configuration works fine on older compilers that don't support this: https://github.com/wiredtiger/wiredtiger/blob/develop/build_posix/configure.ac.in#L110 | |
| Comment by Ryan Egesdahl (Inactive) [ 12/Nov/20 ] | |
|
Unfortunately, it doesn't look like we can do this with our current toolchain. We might be able to do this if and when we update to GCC 8.3 (we're discussing that), but it will almost certainly be able to happen once we get a v4 toolchain. Either way, this will need to wait for a while. | |
| Comment by Alex Cameron (Inactive) [ 19/Oct/20 ] | |
|
Apologies if I've assigned to the wrong team. I'm just guessing since most changes to the top level SConstruct file are from SDP. |