From 128ed87dd8ff5848d4ac1ce905b48bf166c06ff8 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Tue, 12 Dec 2023 16:28:21 +0100 Subject: [PATCH] Reorder members of MapBlock for performance Before and after as obtained via `pahole -C MapBlock bin/minetest`: /* size: 336, cachelines: 6, members: 23 */ /* sum members: 329, holes: 4, sum holes: 7 */ vs. /* size: 336, cachelines: 6, members: 23 */ /* sum members: 329, holes: 2, sum holes: 7 */ There is not much to be gained by packing but I made sure to move the most important data (mainly for the client) into the first cache line. --- src/mapblock.h | 126 ++++++++++++++++++++++++++----------------------- 1 file changed, 66 insertions(+), 60 deletions(-) diff --git a/src/mapblock.h b/src/mapblock.h index 5d87acaed..9a91a05cf 100644 --- a/src/mapblock.h +++ b/src/mapblock.h @@ -437,6 +437,11 @@ public: // clearObject and return removed objects count u32 clearObjects(); + static const u32 ystride = MAP_BLOCKSIZE; + static const u32 zstride = MAP_BLOCKSIZE * MAP_BLOCKSIZE; + + static const u32 nodecount = MAP_BLOCKSIZE * MAP_BLOCKSIZE * MAP_BLOCKSIZE; + private: /* Private methods @@ -444,59 +449,73 @@ private: void deSerialize_pre22(std::istream &is, u8 version, bool disk); -public: /* - Public member variables - */ + * PLEASE NOTE: When adding something here be mindful of position and size + * of member variables! This is also the reason for the weird public-private + * interleaving. + * If in doubt consult `pahole` to see the effects. + */ +public: #ifndef SERVER // Only on client MapBlockMesh *mesh = nullptr; + + // marks the sides which are opaque: 00+Z-Z+Y-Y+X-X + u8 solid_sides = 0; #endif - NodeMetadataList m_node_metadata; - StaticObjectList m_static_objects; - - static const u32 ystride = MAP_BLOCKSIZE; - static const u32 zstride = MAP_BLOCKSIZE * MAP_BLOCKSIZE; - - static const u32 nodecount = MAP_BLOCKSIZE * MAP_BLOCKSIZE * MAP_BLOCKSIZE; - - //// ABM optimizations //// - // Cache of content types - // This is actually a set but for the small sizes we have a vector should be - // more efficient. - // Can be empty, in which case nothing was cached yet. - std::vector contents; - // True if we never want to cache content types for this block - bool do_not_cache_contents = false; - // marks the sides which are opaque: 00+Z-Z+Y-Y+X-X - u8 solid_sides {0}; - private: - /* - Private member variables - */ + // see isOrphan() + bool m_orphan = false; // Position in blocks on parent v3s16 m_pos; - /* This is the precalculated m_pos_relative value - * This caches the value, improving performance by removing 3 s16 multiplications - * at runtime on each getPosRelative call - * For a 5 minutes runtime with valgrind this removes 3 * 19M s16 multiplications - * The gain can be estimated in Release Build to 3 * 100M multiply operations for 5 mins - */ + /* Precalculated m_pos_relative value + * This caches the value, improving performance by removing 3 s16 multiplications + * at runtime on each getPosRelative call. + * For a 5 minutes runtime with valgrind this removes 3 * 19M s16 multiplications. + * The gain can be estimated in Release Build to 3 * 100M multiply operations for 5 mins. + */ v3s16 m_pos_relative; /* - * Note that this is not an inline array because that has implications on + Reference count; currently used for determining if this block is in + the list of blocks to be drawn. + */ + short m_refcount = 0; + + /* + * Note that this is not an inline array because that has implications for * heap fragmentation (the array is exactly 16K), CPU caches and/or * optimizability of algorithms working on this array. */ MapNode *const data; // of `nodecount` elements + // provides the item and node definitions IGameDef *m_gamedef; + /* + When the block is accessed, this is set to 0. + Map will unload the block when this reaches a timeout. + */ + float m_usage_timer = 0; + +public: + //// ABM optimizations //// + // True if we never want to cache content types for this block + bool do_not_cache_contents = false; + // Cache of content types + // This is actually a set but for the small sizes we have a vector should be + // more efficient. + // Can be empty, in which case nothing was cached yet. + std::vector contents; + +private: + // Whether day and night lighting differs + bool m_day_night_differs = false; + bool m_day_night_differs_expired = true; + /* - On the server, this is used for telling whether the block has been modified from the one on disk. @@ -506,14 +525,12 @@ private: u32 m_modified_reason = MOD_REASON_INITIAL; /* - When propagating sunlight and the above block doesn't exist, - sunlight is assumed if this is false. - - In practice this is set to true if the block is completely - undeground with nothing visible above the ground except - caves. + When block is removed from active blocks, this is set to gametime. + Value BLOCK_TIMESTAMP_UNDEFINED=0xffffffff means there is no timestamp. */ - bool is_underground = false; + u32 m_timestamp = BLOCK_TIMESTAMP_UNDEFINED; + // The on-disk (or to-be on-disk) timestamp value + u32 m_disk_timestamp = BLOCK_TIMESTAMP_UNDEFINED; /*! * Each bit indicates if light spreading was finished @@ -525,35 +542,24 @@ private: */ u16 m_lighting_complete = 0xFFFF; - // Whether day and night lighting differs - bool m_day_night_differs = false; - bool m_day_night_differs_expired = true; - - // see isOrphan() - bool m_orphan = false; // Whether mapgen has generated the content of this block (persisted) bool m_generated = false; /* - When block is removed from active blocks, this is set to gametime. - Value BLOCK_TIMESTAMP_UNDEFINED=0xffffffff means there is no timestamp. - */ - u32 m_timestamp = BLOCK_TIMESTAMP_UNDEFINED; - // The on-disk (or to-be on-disk) timestamp value - u32 m_disk_timestamp = BLOCK_TIMESTAMP_UNDEFINED; + When propagating sunlight and the above block doesn't exist, + sunlight is assumed if this is false. - /* - When the block is accessed, this is set to 0. - Map will unload the block when this reaches a timeout. + In practice this is set to true if the block is completely + undeground with nothing visible above the ground except + caves. */ - float m_usage_timer = 0; + bool is_underground = false; - /* - Reference count; currently used for determining if this block is in - the list of blocks to be drawn. - */ - short m_refcount = 0; +public: + NodeMetadataList m_node_metadata; + StaticObjectList m_static_objects; +private: NodeTimerList m_node_timers; };