|
raffopazzo -
I'm going to close this ticket for now. If you pick this project up again, please feel free to re-open this ticket, or, better, open new tickets for the issues you encounter.
|
|
raffopazzo -
Just letting you know that you might want to try again on master. We have made several changes to the build system, and addressed several of the issues identified above.
Please be aware, however, that MIPS is not currently a supported target architecture. You are likely to find other issues, and just because things compile, doesn't necessarily mean that they will run correctly. We do appreciate your filing tickets and raising the issues you discover, however, as they help us identify portability issues which we are making an effort to remove.
|
- For platform detection, there is a
SERVER-9562. Note that support for CCFLAGS on the command line (and many other flags) exists on v3.0 and master as of the commit closing SERVER-16663.
- uclibc: I think local changes would be best.
- V8-3.25, saw the other ticket,
SERVER-17068, thanks for filing that.
- For Atomics, there isn't a ticket exactly for that, but there is a ticket to switch to C++11 everywhere, and that work would roll up there: see
SERVER-16559.
|
|
Many thanks for your comments.
- host. My attempt was rather to lie down a x-platform way of specifying architecture-specific checks or logic.Your idea of specifying CFLAGS etc would surely fit my needs. BTW, may I have the reference to the corresponding JIRA so I can register for updates?
- posix_fallocate. I'll submit the changes in a separate JIRA
- uclibc. I'm currently porting it to MIPS/uClibc-based Set Top Box. So if you ask me, I'd say the uclibc support is necessary. I'm happy to make the changes local to that single file as a separate JIRA.
- V8/Sconscript. Apologies, they slipped in. I'll revert them.
- V8-3.25 missing namespace. I'll address them in a separate JIRA as suggested.
- Atomics. I'll investigate deeper in why the GCC toolchain isn't supporting 64 bits std::atomic. BTW, no particular vendor in this case. We just build the toolchain from GCC sources. You can easily try the same by yourself with crosstool-ng if interested. BTW may I have a reference to the existing JIRA for C++11 atomics so I can register for updates?
- gperftools happy to workaround using the system allocator
Many thanks.
Raffaele.
|
|
OK, I took a high level look. I have some questions and comments, and a suggestion on how to move things forward.
First: I doubt we are going to take everything that you have done, especially not right now or all in one go. I think, overall, that things basically break down into:
- Things we would not have a problem taking.
- Things we aren't 100% sure about either way.
- Things we probably won't want to take.
That said, I think we would like to take as much as possible. If you end up needing to maintain a fork, we should at least make it as small as possible.
In no particular order:
- The 'host' facility and platform detection logic. The build team here is actually looking at this area right now. We want to more or less completely refactor how platform detection works in SCons. It seems that the primary motivation for adding your "host" parameter is so you can switch on the -mips32 or -mips64 flag to the compiler. Is that correct? If so, we probably will not want to take this part of your patch. We are actually hoping to remove much of the platform specific logic, and instead allow customization of the build environment by way of the new ability to set CFLAGS from the command line. Say "scons CCFLAGS=-mips32 LINKFLAGS=-mips32": would that obviate the need for your host flag?
- The change to make detection of posix_fallocate a configure time check. This seems like a good thing to do, period.
- The uclibc changes: Unclear. What system are you porting too that requires this? Is it needed to do anything other than report the libc version? Could we just not do that if it wasn't GNU libc? Do we actually need configure checks and there is no local macro we can example to check inside the version reporting code?
- v8-3.25 MIPS integration: The SCons level changes look, at first glance, OK. But please note that running with v8-3.25 is not a supported mongodb configuration, because V8 removed some critical memory management features. This is why we have not upgraded it to be the default. So, while it is fine to make it work, be aware that it is not a supported build configuration and some important limits on JS execution will not function properly.
- Changes to src/third_party/v8/SConscript: These seem useless if that version of V8 doesn't support MIPS?
- Missing namespace qualifications in v8-3.25 engine files: This was an oversight during a previous refactoring. Please file a separate JIRA ticket stating that 3.0.0-rc6 cannot build with the 3.25 JS engine, and issue those changes as a separate pull request referencing the ticket.
- The changes to the Atomics. I'm afraid that we are very unlikely to take a change like this. Some counters really need to be 64 bits. We are also, as previously noted, moving aggressively to C++11 in the near term and will not be maintaining our own Atomic class, instead relying on std::atomic everywhere, exactly so that we can be more portable. Your changes go against both of these needs.
- gperftools: Needs more research. In the short term, you can work around by not building with tcmalloc on MIPS by using the system allocator. Not ideal, but will work around while we investigate.
I think that is comprehensive but if I missed another area of change please let me know.
So, here is what I propose you do:
- Issue a separate ticket for the V8 namespace qualifications and send us a pull request
- Issue an enhancement ticket for making posix fallocate a configure time check, and send us a pull request
- Decide whether uclibc support is strictly necessary. If so, try to make it a local (file scoped) check, rather than a configure check, since almost all files in the codebase don't care. File a JIRA ticket requesting uclibc support and send us a pull request.
That will leave open:
- Host detection. Hopefully, this gets sorted out soon on our end. Please let us know if you think there is anything we need to be particularly aware of.
- gperftools: Not critical, since there is a workaround.
- Atomics: I propose we re-open this issue once we have moved to C++11. I think you should also work with your toolchain vendor to discuss the lack of 64 bit std::atomic in 32 bit platforms: this sounds like a compiler bug or quality of implementation issue. At the very least they should be able to offer lock based implementations of 64-bit counters in 32-bit mode, which, as previously discussed, is not a platform expected to function well for production workloads.
Finally, please note that we are currently in a code freeze as we work towards the upcoming 3.0.0 release, so any changes will need to wait for the merge window to reopen.
In any event, we can definitely take a good amount of the work you have done and slim down the amount you would need to maintain, short term, until we work out the longer term issues. It is also important to be aware that we do not have access to MIPS testing systems so we will not be able to ensure that we continue to be portable to MIPS, uclibc, or other variations, or offer support for deployments on such platforms.
Thanks,
Andrew
|
|
Hi acm
I believe I've almost finished the work, even though it's not ready to raise a pull request yet.
However, I was wondering if you guys mind having a look at it, kind of a draft review so we can make sure I've not gone completely in the wrong direction.
This is my github fork: https://github.com/raffopazzo/mongo/tree/mips
Many thanks.
Raf.
|
|
I've just hit the issue with WiredTiger. For the time being I've just disabled it with --wiredtiger=off. I wonder what would be the consequences, particularly for embedded systems with no 32 bits atomics, which is what this activity is addressing. For these kind of systems it's highly unlikely that the limit of 2GB of data is a problem anyway, and still an improvement of not having mongo at all.
What's your opinion on this? What is WiredTiger for? Do you reckon acceptable for these systems to voluntarily switch off WiredTiger?
As a matter of fact, SConstruct is currently switching it off in case CheckForx86() passes. So I don't think you would disagree 
Many thanks.
Raf.
|
|
As far as 32 bits go, please keep in mind that 32-bits is not a supported runtime environment for mongodb. We really only offer it so that people can do some very limited experiments on 32-bit linux and windows machines. It is not a supported production deployment environment: on a 32-bit machine MongoDB is limited to 2GB of data in the database due to the use of mmap, and the new WiredTiger storage engine is 64-bits only. So, changes that make things more complex in an effort to improve 32 bit support are unlikely to be viewed positively.
|
|
Hi acm
- I'm doing this on master. When I did it months ago it still was master. It was around last July I think, and I'm seeing the same issues. So I'd imagine nothing major's changes where MIPS is concerned
- I'll keep looking into the GCC. I'm using GCC 4.9. I'll try build several toolchains to check whether it'll behave differently but I'm very inclined to believe that what the manual page means is that mips64 supports 8 bytes atomic.
- Nice. I'll start from there. Btw, Just out of curiosity, I'd like to understand why you guys chose scons instead of autotools.
Thank you very much.
Raf.
|
|
Hi raffopazzo -
- I forgot to ask what version of MongoDB you are attempting to port. 2.6? Master?
- As far as std::atomic, the GCC wiki page on Atomics suggests that MIPS is a fully supported architecture, for word sizes up to 8 bytes (see https://gcc.gnu.org/wiki/Atomic). What version of GCC are you using?
- Notice that we don't actually run configure for tcmalloc. For better or worse, the current approach is that we have captured config.h headers for different platforms by manually running the gperftools configure script on those platforms. See src/third_party/gperftools-2.2/src/config-10gen*. You likely need to do the same for MIPS.
Thanks,
Andrew
|
|
Hi acm, many thanks for your comments.
- I had quick go to std:;atomic<> and despite the reference lets us believe that the C++ should provide 64 bits atomic operation even if they're not lock-free, it seems that GCC for mips still doesn't do that. So I'm not confident C++11 would solve the problem for free. I'd be tempted to do the change I've got in mind and then let you review it. I'll then be happy to change any thing you guys don't see fit.
- vanilla gperftools-2.2 seems to support MIPS but I'm trying to understand what's going wrong with the porting in the mongo codebase. Perhaps it's due to the way you configure-ed it ? Any ideas?
Many thanks.
Raf.
|
|
Hi raffopazzo - I have several comments:
- The process/platform detection logic in the top level SConstruct is something that we are hoping to significantly refactor in the near term. I'd caution you away from making sweeping changes here since they are likely to conflict with upcoming work.
- New MONGO_HAVE defines are fine. If you add one, please carefully follow the structure of existing configure checks. Note that we may, in the future, migrate many of the HAVE definitions into a SusbtFile generated config.h.
- I'm quite concerned about the situation with Atomics. I think we would be dis-inclined to accept changes migrating 64 bit counters away from 64 bits. In addition, we are going to be, in the very near future, migrating to C++11, where the only acceptable vehicle for atomic operations will be the std::atomic types. A conforming implementation, must, I believe provide a specialization of std::atomic for 64-bit integer types, even if the underlying support is not lock-free (see http://en.cppreference.com/w/cpp/atomic/atomic/is_lock_free). You might find that just waiting for the switch to C++11 lets you sidestep the issue entirely. Obviously, the 64-bit counters would be slower, but if you are targeting a 32-bit platform you are already running MongoDB in a degraded capacity. Presumably, on 64-bit MIPS, the 64-bit types have efficient atomic implementations.
- Your mention of gperftools raises a separate concern, which is the portability of things like v8 to non-x86 platforms. Extensive changes to third_party libraries are disfavored as it makes it more difficult for us to migrate to newer versions or integrate bug fixes.
Overall, I would encourage you to try extremely hard to make your changes as minimal as possible, erring on the side of degraded behavior/performance on MIPS. Even so, we still cannot offer any assurances that we will be able to merge your changes.
Please note that we are very interested in making MongoDB run correctly on a wider range of platforms, but often there are non-obvious technical obstacles to doing so that must be cleared before we can make progress.
Thanks,
Andrew
Andrew
|
|
So I have identified the following changes:
in top-level SConstruct
- I'd introduce a host argument which could be set to the target arch name like mips (or alternatively a GNU-like platform triplet like mips-linux-uclibc). This would allow to do platform-specific checks like if force32: processor = "i386" or endianess detection only if the check is meaningful for that particular host
- A new check may need to be introduced to detect whether the C library provide posix_fallocate() currently used in FileAllocator.cpp and this would lead to a new MONGO_HAVE_POSIX_FALLOCATE macro to #ifdef against
The great number of changes would anyway be in the code where Counter64 and AtomicInt64 and similar are used, as not all architectures supports 64 bits atomic operations. All these changes would be all identical anyway, and a first suggestion could be to profile all usage of these 64 bits atomic type with some sort of MONGO_HAVE_64_BITS_ATOMIC to be tested by SConstruct.
For example
|
src/mongo/client/dbclientinterface.h
|
#ifdef MONGO_HAVE_64_BITS_ATOMIC
|
static AtomicInt64 ConnectionIdSequence;
|
#else
|
static AtomicInt32 ConnectionIdSequence;
|
#endif
|
To be honest, pretty much all of these atomic integers seems to be used as bare counters. In which case a generic AtomicCounter would minimise the number of #ifdef-s. This AtomicCoutner could simply be an alias to for the biggest available atomic integer. Or similar. Please share ideas.
Then I'd need to imlpement strackrace inspection in src/third_party/gperftools-2.2/src/stacktrace.cc which I don't know much yet.
Is this the right level of details you was expecting? Do you need something different?
Thanks
|
|
Comments would be fine.
|
|
Will posting them here as comments be ok ?
It seems I can't modify the ticket description anymore or at least I can't work out how 
|
|
raffopazzo -
I'd recommend that you write up a detailed design for what you intend to do and post it in this ticket. We have, over the years, received many pull requests adding portability enhancements for various platforms that we have been unable to merge. Often, the approach is either too narrow and only fixes cross platform issues for one specific platform, or too broad, and makes sweeping changes that conflict with internal plans or constraints.
Thanks,
Andrew
|
|
Some time ago I did fix this (in a pretty hacky way) and I cross-compiled it for mips/uclibc. Now I'm doing it again in a cleaner way so to make it available for the community. Shall someone assign it to me ? Also is there an agreed way of discussing changes before doing them or will all the changes be reviewed only when I submit a patch/pull request ?
|
Generated at Thu Feb 08 03:42:39 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.