From 0e43ca65681e09fe4672d36ec0147a9f1c57c060 Mon Sep 17 00:00:00 2001 From: jyelon Date: Mon, 25 Mar 2024 17:37:58 -0400 Subject: [PATCH] Some documentation improvements --- luprex/cpp/core/animqueue.hpp | 18 ++++++++++++++- luprex/cpp/core/eng-malloc.hpp | 41 +++++++++++++++++++++++++--------- 2 files changed, 48 insertions(+), 11 deletions(-) diff --git a/luprex/cpp/core/animqueue.hpp b/luprex/cpp/core/animqueue.hpp index 4cec99e8..6f08d677 100644 --- a/luprex/cpp/core/animqueue.hpp +++ b/luprex/cpp/core/animqueue.hpp @@ -171,7 +171,7 @@ public: // Add default values for all builtin persistent variables. // - // For each builtin default (plane, xyz, facing, bp, model) + // For each builtin default (plane, xyz, facing, bp) // // - Will generate an error if a value is already present, // but the present value is of the wrong type. @@ -224,6 +224,22 @@ public: // Parse a string, for unit testing. // + // This parses a simple notation designed to facilitate writing unit tests. + // The notation looks like this: + // + // parse("plane=earth xyz=1,2,3 action:jump"); + // + // Determining the type of the value is done as follows: First, try + // interpreting it as boolean true or false. If that fails, + // try interpreting it as a number. If that fails, try interpreting it as + // a coordinate. If all else fails, it's a string. Obviously, this is a + // limited approach: for example, there's no way to express the string "123" + // because "123" will get interpreted as a number. But that's ok, since this + // is purely intended for unit testing. + // + // Key-value pairs can have either an equal sign or a colon. If it's an equal + // sign, the persistent bit is set, colon means not persistent. + // void parse(std::string_view s); void clear_and_parse(std::string_view s); diff --git a/luprex/cpp/core/eng-malloc.hpp b/luprex/cpp/core/eng-malloc.hpp index 27a06d78..fe33b9ca 100644 --- a/luprex/cpp/core/eng-malloc.hpp +++ b/luprex/cpp/core/eng-malloc.hpp @@ -5,6 +5,13 @@ // everything. The engine's malloc heap only contains engine data structures. // This helps achieve determinism when playing a replay log. // +// About determinism: one of the key rules for maintaining deterministic +// behavior is to not ever use operations that execute in arbitrary order. +// For example, don't iterate over an unordered map, because there's no +// rule about what order the items are produced. They could be produced +// in a different order during replay than during the original recording. +// Actually, you can occasionally get away with it +// // The engine's eng::malloc is a thin wrapper around Doug Lea's Malloc, a good // general-purpose single-threaded malloc. It's probably not the fastest any // more (it was, once), but it's still quite good. It's also fairly easy @@ -13,9 +20,11 @@ // In order to get all engine data structures into the eng::malloc heap, you // need to jump through quite a few hoops: // -// * When writing a class, always derive from eng::opnew. This adds a -// custom operator new to your class, which causes your class to be -// allocated using eng::malloc. +// * When writing a class that gets allocated using operator new, +// always derive from eng::opnew. This adds a custom operator new +// to your class, which causes your class to be allocated using eng::malloc. +// If you write a class that isn't ever supposed to be allocated using +// operator new, derive from eng::nevernew instead. // // * When using STL containers, you need to use the eng variant: // eng::map, eng::set, eng::vector, eng::unordered_map, eng::unordered_set, @@ -26,20 +35,32 @@ // instead of std::ostringstream. // // * Simple classes like std::pair, std::string_view, std::less, std::hash, and -// so forth are not wrapped. Do not use operator new or delete on +// so forth are not wrapped, because it is not normal to allocate these +// classes using operator new. Do not use operator new or delete on // these classes. // // * Instead of std::make_shared, use eng::make_shared. You need this // because std::make_shared doesn't respect your custom operator new. // -// * Be aware that most C++ streams use the system malloc heap, and there's no -// way to change that. Fortunately, eng::ostringstream uses the eng::malloc -// heap. -// // * Failing to jump through all these hoops won't break your code in any // obvious way - you'll just have some of your data structures in the malloc -// heap instead of the eng::malloc heap. This can break determinism of -// replay. +// heap instead of the eng::malloc heap. This won't break +// determinism unless you iterate over a data structure like an unordered map +// + + +but it creates a situation where we can't detect +// nondeterminism. +// +// * Sometimes we deliberately put certain data structures into the malloc +// heap, because we know that those particular data structures won't be +// identical between record and replay. In that situation, the fact that +// we don't detect the nondeterminism is actually a benefit. +// +// * Be aware that most C++ streams use the system malloc heap, and there's no +// way to change that. That's ok, it's fine if some small percentage of our +// data goes into the malloc heap. By the way, eng::ostringstream uses +// the eng::malloc heap. // #ifndef ENG_MALLOC_HPP #define ENG_MALLOC_HPP