[SERVER-43175] update mongo/platform/endian.h Created: 05/Sep/19  Updated: 29/Oct/23  Resolved: 03/Oct/19

Status: Closed
Project: Core Server
Component/s: Internal Code
Affects Version/s: None
Fix Version/s: 4.3.1

Type: Improvement Priority: Major - P3
Reporter: Billy Donahue Assignee: Billy Donahue
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Backwards Compatibility: Fully Compatible
Sprint: Dev Tools 2019-09-23, Dev Tools 2019-10-07
Participants:

 Description   

This is a very low-level bare metal header, and should ideally #include only std headers.

  • Decimal128 doesn't need endian conversions, and makes endian.h transitively include several higher-level mongo specifics. Current conversions are underspecified and mathematically incorrect or at least ambiguous. They swap the order within each of the low64 and high64 fields, but don't swap them with each other. This is behavior needed only by one spot in db/pipeline/value.cpp to deserialize NumberDecimal, so we can just inline the behavior there and remove it from endian.h.
  • The running compiler holds the ultimate truth on what the target endianness is (available via _BYTE_ORDER_). We should not read it from a config header. The names exported into C++ code for the endian possibilities can be changed to line up with those in C++20's std::endian {big,little,native}

    enum. Remove MONGO_CONFIG_BYTE_ORDER from config.h. This eliminates the Scons<=>C++ bridge protocol of "1234" and "4321" magic numbers. Then Scons only talks to the compiler, not to the code.

  • Use enum expressions (including if constexpr) rather than #if for branching on endianness. This makes bit-rot of unexecuted paths less likely, and is just cleaner C++.
  • endian.h supports noisy old #if//#elif/... branches for getting compiler-builtin bswap operations. Can reduce to a simple MSVC vs GCC branching. All compilers have a builtin, so remove the bswap_slow" implementation.
  • Don't need all the push_macro / pop_macro stuff, or really any macros at all. Just rely on inline C++ functions. These optimize to the same thing. Don't need ByteOrderConverter or IntegralTypeMap either. Simpler metaprogramming will work fine. All in all we can utterly remove 400 lines of code there.


 Comments   
Comment by Githook User [ 02/Oct/19 ]

Author:

{'name': 'Billy Donahue', 'username': 'BillyDonahue', 'email': 'billy.donahue@mongodb.com'}

Message: SERVER-43175 platform/endian.h refresh

This is a very low-level bare metal header, and should ideally
#include only std headers.

  • Remove Decimal128 dependency from endian.h. Decimal128 doesn't
    need endian conversions, and makes endian.h transitively include
    several higher-level mongo specifics. Current conversions are
    underspecified and mathematically incorrect or at least ambiguous.
    They swap the order within each of the low64 and high64 fields,
    but don't swap them with each other. This is behavior needed only
    by one spot in db/pipeline/value.cpp to deserialize NumberDecimal,
    so we can just inline the behavior there and remove it from
    endian.h.
  • Remove MONGO_CONFIG_BYTE_ORDER from config.h. The running compiler
    holds the ultimate truth on what the target endianness is
    (available via BYTE_ORDER). We should not read it from a config
    header. The names exported into C++ code for the endian
    possibilities can be changed to line up with those in C++20's
    std::endian {big,little,native}

    enum. This eliminates the
    Scons<=>C++ bridge protocol of "1234" and "4321" magic numbers.
    Scons will talk to the compiler, not directly to the code.

  • Use enum expressions (including if constexpr) rather than #if for
    branching on endianness. This makes bit-rot of unexecuted paths
    less likely, and is just cleaner C++.
  • Remove bswap_slow variants. All supported compilers have builtin
    bswap operations. Can reduce to a simple MSVC vs GCC branching.
    All compilers have a builtin, so remove the bswap_slow"
    implementation.
  • Don't need all the push_macro / pop_macro stuff, or really any
    macros at all. Just rely on inline C++ functions. These optimize to
    the same thing.
  • Don't need ByteOrderConverter or IntegralTypeMap traits either.
    Simpler metaprogramming based only on sizeof will work fine.

All in all we can remove about 400 lines of old code here and
shave some low-level edges off of the dependency graph.

Comment by Billy Donahue [ 18/Sep/19 ]

https://mongodbcr.appspot.com/499180013/

Generated at Thu Feb 08 05:02:28 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.