Eager caching with parse hints (native-layer only)

Author: kylereedmsft

krabs/krabs/parser.hpp

@@ -78,6 +78,15 @@ namespace krabs {
         template <typename T>
         bool try_parse(std::wstring_view name, T &out);
 
+        /**
+         * <summary>
+         * Attempts to retrieve the given property by name and type,
+         * starting the name scan at the given hint index.
+         * </summary>
+         */
+        template <typename T>
+        bool try_parse(std::wstring_view name, T &out, ULONG hint);
+
         /**
          * <summary>
          * Attempts to parse the given property by name and type. If the
@@ -87,22 +96,36 @@ namespace krabs {
         template <typename T>
         T parse(std::wstring_view name);
 
+        /**
+         * <summary>
+         * Attempts to parse the given property by name and type,
+         * starting the name scan at the given hint index.
+         * </summary>
+         */
+        template <typename T>
+        T parse(std::wstring_view name, ULONG hint);
+
         template <typename Adapter>
         auto view_of(std::wstring_view name, Adapter &adapter) -> collection_view<typename Adapter::const_iterator>;
 
+        template <typename Adapter>
+        auto view_of(std::wstring_view name, ULONG hint, Adapter &adapter) -> collection_view<typename Adapter::const_iterator>;
+
     private:
         property_info find_property(std::wstring_view name);
-        void cache_property(ULONG index, property_info info);
+        property_info find_property(std::wstring_view name, ULONG hint);
+        void ensure_cache_populated();
 
     private:
         const schema &schema_;
         const BYTE *pEndBuffer_;
-        BYTE *pBufferIndex_;
-        ULONG lastPropertyIndex_;
-        // Persistent name to index map shared across all events of the same type.
-        const property_name_map *pPropertyNames_;
-        // Maintain a mapping from property index to blob data location.
+
+        // Fully populated on first access -- maps property index to its
+        // location and size in the event's user-data blob.
         std::vector<property_info> propertyCache_;
+
+        // Hint for name scan -- start from here on the next lookup.
+        ULONG nextHint_;
     };
 
     // Implementation
@@ -111,112 +134,88 @@ namespace krabs {
     inline parser::parser(const schema &s)
     : schema_(s)
     , pEndBuffer_((BYTE*)s.record_.UserData + s.record_.UserDataLength)
-    , pBufferIndex_((BYTE*)s.record_.UserData)
-    , lastPropertyIndex_(0)
-    , pPropertyNames_(s.pPropertyNames_)
-    , propertyCache_(s.pSchema_->PropertyCount)
+    , nextHint_(0)
     {}
 
     inline property_iterator parser::properties() const
     {
         return property_iterator(schema_);
     }
 
-    inline property_info parser::find_property(std::wstring_view name)
+    inline void parser::ensure_cache_populated()
     {
-        // A schema contains a collection of properties that are keyed by name.
-        // These properties are stored in a blob of bytes that needs to be
-        // interpreted according to information that is packaged up in the
-        // schema and that can be retrieved using the Tdh* APIs. This format
-        // requires a linear traversal over the blob, incrementing according to
-        // the contents within it. This is janky, so our strategy is to
-        // minimize this as much as possible via caching.
-
-        const ULONG totalPropCount = schema_.pSchema_->PropertyCount;
-
-        // Resolve property name to index.
-        ULONG index = totalPropCount; // sentinel = not found
-        if (pPropertyNames_) {
-            // Fast path: use the persistent name to index map shared across
-            // all events of the same type.
-            auto it = pPropertyNames_->find(name);
-            if (it != pPropertyNames_->end()) {
-                index = it->second;
-            }
-        } else {
-            // Fallback: linear scan of property names in the schema.
-            for (ULONG i = 0; i < totalPropCount; ++i) {
-                auto &propInfo = schema_.pSchema_->EventPropertyInfoArray[i];
-                const wchar_t *pName = reinterpret_cast<const wchar_t*>(
-                    reinterpret_cast<const BYTE*>(schema_.pSchema_) +
-                    propInfo.NameOffset);
-                if (name == pName) {
-                    index = i;
-                    break;
-                }
-            }
-        }
-
-        if (index >= totalPropCount) {
-            return property_info();
+        if (!propertyCache_.empty()) {
+            return;
         }
 
-        // The first step is to use our cache for the property to see if we've
-        // discovered it already.
-        if (index < lastPropertyIndex_) {
-            return propertyCache_[index];
+        const ULONG totalPropCount = schema_.pSchema_->PropertyCount;
+        if (totalPropCount == 0) {
+            return;
         }
 
-        assert((pBufferIndex_ <= pEndBuffer_ && pBufferIndex_ >= schema_.record_.UserData) &&
-               "invariant: we should've already thrown for falling off the edge");
-
-        // accept that last property can be omitted from buffer. this happens if last property
-        // is string but empty and the provider strips the null terminator
-        assert((pBufferIndex_ == pEndBuffer_ ? ((totalPropCount - lastPropertyIndex_) <= 1)
-                                             : true)
-               && "invariant: if we've exhausted our buffer, then we must've"
-                  "exhausted the properties as well");
+        propertyCache_.reserve(totalPropCount);
+        BYTE *pBuffer = (BYTE*)schema_.record_.UserData;
 
-        // We've not looked up this property before, so we have to do the work
-        // to find it. While we're going through the blob to find it, we'll
-        // remember what we've seen to save time later.
-        //
-        // Note: The name-to-index map is built once per schema type (cheap
-        // metadata scan). But the blob walk below is lazy per-event -- we
-        // only walk forward to the requested index, avoiding overhead when
-        // only a subset of properties are needed.
-        while (lastPropertyIndex_ <= index) {
-
-            auto &currentPropInfo = schema_.pSchema_->EventPropertyInfoArray[lastPropertyIndex_];
+        for (ULONG i = 0; i < totalPropCount; ++i) {
+            auto &currentPropInfo = schema_.pSchema_->EventPropertyInfoArray[i];
             const wchar_t *pName = reinterpret_cast<const wchar_t*>(
                                         reinterpret_cast<const BYTE*>(schema_.pSchema_) +
                                         currentPropInfo.NameOffset);
 
             ULONG propertyLength = size_provider::get_property_size(
-                                        pBufferIndex_,
+                                        pBuffer,
                                         pName,
                                         schema_.record_,
                                         currentPropInfo);
 
-            // verify that the length of the property doesn't exceed the buffer
-            if (pBufferIndex_ + propertyLength > pEndBuffer_) {
+            if (pBuffer + propertyLength > pEndBuffer_) {
                 throw std::out_of_range("Property length past end of property buffer");
             }
 
-            property_info propInfo(pBufferIndex_, currentPropInfo, propertyLength);
-            cache_property(lastPropertyIndex_, propInfo);
-
-            // advance the buffer index since we've already processed this property
-            pBufferIndex_ += propertyLength;
-            lastPropertyIndex_++;
+            propertyCache_.emplace_back(pBuffer, currentPropInfo, propertyLength);
+            pBuffer += propertyLength;
         }
+    }
 
-        return propertyCache_[index];
+    inline property_info parser::find_property(std::wstring_view name)
+    {
+        return find_property(name, nextHint_);
     }
 
-    inline void parser::cache_property(ULONG index, property_info info)
+    inline property_info parser::find_property(std::wstring_view name, ULONG hint)
     {
-        propertyCache_[index] = info;
+        ensure_cache_populated();
+
+        const ULONG totalPropCount = schema_.pSchema_->PropertyCount;
+        if (totalPropCount == 0) {
+            return property_info();
+        }
+
+        // Hinted linear scan. In the common case (sequential access
+        // or caller-provided index) this hits on the first comparison.
+        if (hint >= totalPropCount) {
+            hint = 0;
+        }
+
+        ULONG index = totalPropCount; // sentinel = not found
+        for (ULONG n = 0; n < totalPropCount; ++n) {
+            ULONG i = (hint + n) % totalPropCount;
+            auto &propInfo = schema_.pSchema_->EventPropertyInfoArray[i];
+            const wchar_t *pName = reinterpret_cast<const wchar_t*>(
+                reinterpret_cast<const BYTE*>(schema_.pSchema_) +
+                propInfo.NameOffset);
+            if (name == pName) {
+                index = i;
+                break;
+            }
+        }
+
+        if (index >= totalPropCount) {
+            return property_info();
+        }
+
+        nextHint_ = (index + 1) % totalPropCount;
+        return propertyCache_[index];
     }
 
     inline void throw_if_property_not_found(const property_info &propInfo)
@@ -244,6 +243,13 @@ namespace krabs {
     // try_parse
     // ------------------------------------------------------------------------
 
+    template <typename T>
+    bool parser::try_parse(std::wstring_view name, T &out, ULONG hint)
+    {
+        nextHint_ = hint;
+        return try_parse(name, out);
+    }
+
     template <typename T>
     bool parser::try_parse(std::wstring_view name, T &out)
     {
@@ -269,6 +275,13 @@ namespace krabs {
     // parse
     // ------------------------------------------------------------------------
 
+    template <typename T>
+    T parser::parse(std::wstring_view name, ULONG hint)
+    {
+        nextHint_ = hint;
+        return parse<T>(name);
+    }
+
     template <typename T>
     T parser::parse(std::wstring_view name)
     {
@@ -435,6 +448,14 @@ namespace krabs {
     // view_of
     // ------------------------------------------------------------------------
 
+    template <typename Adapter>
+    auto parser::view_of(std::wstring_view name, ULONG hint, Adapter &adapter)
+        -> collection_view<typename Adapter::const_iterator>
+    {
+        nextHint_ = hint;
+        return view_of(name, adapter);
+    }
+
     template <typename Adapter>
     auto parser::view_of(std::wstring_view name, Adapter &adapter)
         -> collection_view<typename Adapter::const_iterator>

krabs/krabs/schema.hpp

@@ -308,8 +308,6 @@ namespace krabs {
     private:
         const EVENT_RECORD &record_;
         const TRACE_EVENT_INFO *pSchema_;
-        // Persistent name to index map, owned by schema_locator. May be nullptr.
-        const property_name_map *pPropertyNames_;
 
     private:
         friend std::wstring event_name(const schema &);
@@ -339,13 +337,11 @@ namespace krabs {
     inline schema::schema(const EVENT_RECORD &record, const krabs::schema_locator &schema_locator)
         : record_(record)
         , pSchema_(schema_locator.get_event_schema(record))
-        , pPropertyNames_(schema_locator.get_property_names(pSchema_))
     { }
 
     inline schema::schema(const EVENT_RECORD &record, const PTRACE_EVENT_INFO pSchema)
         : record_(record)
         , pSchema_(pSchema)
-        , pPropertyNames_(nullptr)
     { }
 
     inline bool schema::operator==(const schema &other) const

krabs/krabs/schema_locator.hpp

@@ -173,14 +173,6 @@ namespace krabs {
      */
     std::string_view get_trace_logger_event_name(const EVENT_RECORD &);
 
-    /**
-     * <summary>
-     * Maps property names to their index in the schema.
-     * Keys are wstring_views pointing into stable TRACE_EVENT_INFO memory.
-     * </summary>
-     */
-    using property_name_map = std::unordered_map<std::wstring_view, ULONG>;
-
     /**
      * <summary>
      * Fetches and caches schemas from TDH.
@@ -215,21 +207,8 @@ namespace krabs {
          */
         bool has_event_schema(const EVENT_RECORD& record) const;
 
-        /**
-         * <summary>
-         * Returns the persistent property name to index map for a schema.
-         * The map is built when the schema is first cached.
-         * Returns nullptr if pSchema is null or not in the cache.
-         * </summary>
-         */
-        const property_name_map* get_property_names(const TRACE_EVENT_INFO* pSchema) const;
-
     private:
-        void build_property_names(const TRACE_EVENT_INFO* pSchema) const;
-
         mutable std::unordered_map<schema_key, std::variant<std::unique_ptr<char[]>, TDHSTATUS>> cache_;
-        // Persistent property name to index maps, keyed by schema pointer.
-        mutable std::unordered_map<const TRACE_EVENT_INFO*, property_name_map> property_name_cache_;
     };
 
     // Implementation
@@ -332,10 +311,9 @@ namespace krabs {
 
         // Add the new instance to the cache.
         // NB: key's 'internalize_name' gets called by the cctor here.
-        if (status == ERROR_SUCCESS) {
+        if (status == ERROR_SUCCESS)
             cache_.emplace(key, std::move(buffer));
-            build_property_names(returnVal);
-        } else
+        else
             cache_.emplace(key, status);
 
         return returnVal;
@@ -348,29 +326,6 @@ namespace krabs {
         return status == ERROR_SUCCESS;
     }
 
-    inline void schema_locator::build_property_names(const TRACE_EVENT_INFO* pSchema) const
-    {
-        property_name_map names;
-        for (ULONG i = 0; i < pSchema->PropertyCount; ++i) {
-            const wchar_t* pName = reinterpret_cast<const wchar_t*>(
-                reinterpret_cast<const BYTE*>(pSchema) +
-                pSchema->EventPropertyInfoArray[i].NameOffset);
-            names.emplace(std::wstring_view(pName), i);
-        }
-        property_name_cache_.emplace(pSchema, std::move(names));
-    }
-
-    inline const property_name_map* schema_locator::get_property_names(const TRACE_EVENT_INFO* pSchema) const
-    {
-        if (!pSchema) return nullptr;
-
-        auto it = property_name_cache_.find(pSchema);
-        if (it != property_name_cache_.end()) {
-            return &it->second;
-        }
-        return nullptr;
-    }
-
     inline std::unique_ptr<char[]> get_event_schema_from_tdh(const EVENT_RECORD &record)
     {
         TDHSTATUS status = ERROR_SUCCESS;