Danfeng Wang

This commit introduces a **new capability** by adding `getMethodMetadata()` to **Thrift-generated processors** in the Hack/PHP runtime, enabling more robust **service method dispatch** with proper **inheritance chain delegation**. It refactors `ThriftAsyncProcessor` to utilize this new metadata system, which includes new interfaces and classes for handling method details, and updates the visibility of a helper method in `ThriftProcessorBase`. Extensive **unit tests** are included to verify correct metadata retrieval across service inheritance hierarchies, as well as for streaming and sink methods, ensuring the reliability of this core dispatch mechanism. This enhancement improves the maintainability and clarity of how Thrift services resolve and execute methods, especially in complex inherited service structures.

This commit **refactors** the **Thrift compiler's Hack generator** to improve how method metadata is produced for async processors. It now generates a single `getMethodMetadata` function with an inlined switch statement, replacing the previous pattern of individual method metadata functions. This change centralizes IDL-dependent code generation, enhancing the separation from IDL-independent base class logic within **Thrift service implementations**. The update streamlines the generated code for **Thrift async processors**, leading to a more organized and potentially efficient approach to RPC request handling.

This commit **refactors** the **Thrift compiler's Hack generator** by adjusting the access modifier of the `getMethodMetadata_` functions. Specifically, it changes the visibility of these generated metadata functions from public to **protected** within `t_hack_generator.cc`. This **access modifier adjustment** improves **encapsulation** and code hygiene, ensuring that these internal utility functions are only accessible by the class and its derived classes. The change primarily affects the generated Hack code's internal structure, promoting better modularity.

This commit introduces a **new Thrift processor architecture** within the `ThriftAsyncProcessor` module, specifically for **Hack Thrift services**. It adds new classes and methods, such as `getMethodMetadata*` functions, to define comprehensive method metadata for various **Thrift RPC types**, including request-response, oneway, streaming, and sink methods. This **architectural enhancement** semantically separates IDL-dependent generated code from the core `processAsync` logic, improving modularity and maintainability. The change primarily impacts the **Thrift runtime for Hack**, enabling more flexible and robust handling of diverse RPC patterns.

This commit introduces a **new capability** to the **Hack Thrift code generator**, enabling it to generate `getMethodMetadata*` functions within `thrift/compiler/generate/t_hack_generator.cc`. These functions are crucial for the **processor base class**, providing essential metadata such as argument classes, result classes, and handler lambdas for various method types. The generation specifically covers **request-response, streaming, sink, and oneway methods**. For **oneway methods**, the generation is optimized to use a simplified `ThriftServiceOnewayMethod` and explicitly skips the creation of unnecessary `_result` structs. This enhancement streamlines how method metadata is exposed and consumed within the Hack Thrift runtime.

This commit implements a **performance optimization** for **Python Thrift map deserialization** by introducing a `has_unicode_error` flag. The deserialization logic in `thrift/lib/python/types.cpp` and `thrift/lib/cpp2/protocol` is enhanced to detect when string keys or values in a map fall back to bytes due to UTF-8 decoding failures. This **new capability** allows downstream Python code to check the `ImmutableInternalMap.has_unicode_error` flag, enabling it to **skip unnecessary validation** for maps guaranteed to contain only Unicode strings. Consequently, this change delivers **significant performance improvements**, making first-time iteration for string-based maps **2.3x to 2.5x faster**.

This commit **introduces new performance benchmarks** for the **JSON protocol** within the `thrift/lib/python/benchmark` module. It adds dedicated tests for **serialization and deserialization** of JSON protocol messages, complementing the existing Compact protocol benchmarks. This **new capability** allows for direct performance comparison across all Python Thrift flavors (`py-deprecated`, `py3`, `python`, `mutable-python`), providing crucial data for understanding protocol overhead. The initial results highlight that the JSON protocol is significantly slower than the Compact protocol, offering valuable insights for performance optimization.

This commit **optimizes** the **Thrift Python `Map` type's lazy conversion mechanism** by **removing caching** from the `_fbthrift_lazy_getitem()` method. Previously, accessed values were converted and cached, but now they are re-converted on each access, simplifying the internal structure and reducing overhead. This **refactoring** primarily affects the **`thrift.lib.python.types` module** and its `Map` class, leading to **significant performance improvements** (up to 48% faster) for random access operations. The `__getitem__` method is also **simplified** to directly check the lazy flag, further enhancing efficiency by avoiding `try-except` overhead.

This commit **refactors** the **Thrift Python runtime** by strengthening the internal type declarations for **Map** data structures. It updates the `cdef` declarations for internal storage fields like `_fbthrift_elements` and `_fbthrift_internal_elements` in `thrift/lib/python/types.pxd` from generic `object` to specific `dict` types. This **performance optimization** allows Cython to generate more efficient C code, directly utilizing `PyDict_*` functions instead of generic `PyObject_*` APIs for dictionary operations. Explicit `<dict>` casts were added in `thrift/lib/python/types.pyx` to maintain type safety during assignments, ensuring **improved performance** for `Map` instances without altering their external behavior.

This commit **refactors** the **Thrift Python library's type handling** by **removing the use of `types.MappingProxyType`** when assigning map elements. Specifically, the `__init__` method within `thrift/lib/python/types.pyx` no longer wraps map elements with this proxy. This change is a **performance optimization** aimed at evaluating whether avoiding the `MappingProxyType` overhead can significantly **improve runtime performance** within the `thrift/lib/python` module. The `thrift/lib/python/types.pxd` file is also updated to reflect this change in its comments, indicating a shift away from using `MappingProxyType` for map elements. This modification prioritizes potential speed gains over the immutability provided by the proxy.

This commit introduces **lazy, in-place validation** for map elements within the **Thrift Python runtime**, specifically modifying the `Map` class in `thrift/lib/python/types.pyx`. This **new feature** adds helper functions like `supports_in_place_validation` to check if both key and value types can be validated without conversion. The change significantly **improves performance** by avoiding dictionary reconstruction for maps with types like `i64` or `string`, where elements can be validated in-place. This optimization builds upon previous work for set validation, enhancing the efficiency of Thrift data processing.

This commit introduces a significant **performance optimization** to the **Thrift Python runtime library** by implementing **lazy value conversion** for map types. Specifically, for maps with primitive keys (like `int`, `float`, or `str`), the conversion of map values from their internal representation to Python types is now deferred until the value is actually accessed, rather than happening upfront. This **new capability** primarily affects the `Map` class, modifying its internal storage (`_fbthrift_internal_elements`, `_fbthrift_elements`) and access methods (`__getitem__`, `keys`, `values`, `items`) to support this on-demand conversion. The change results in substantial **performance improvements**, achieving up to **30x faster random access** for common use cases by avoiding unnecessary type conversions.

This commit introduces a significant **performance improvement** and **refactoring** to the **Thrift Python runtime** by changing the internal representation of `map` data types. Specifically, it converts the internal storage from a `tuple` of tuples to a custom `ImmutableInternalMap` (a hashable `dict`), which drastically reduces overhead for random access and first-iteration performance. This optimization primarily benefits `map`s where both keys and values are **Thrift Primitive Types** (excluding `string`), leading to up to **60x faster random access** and **2.6x faster first iteration** for types like `map<i32, i32>`. The changes are implemented across `types.pyx`, `types.cpp`, `constructor.h`, and `extractor.h`, ensuring that the user-facing Python `Map` type remains consistent while enhancing the efficiency of internal serialization and deserialization.

This commit introduces **lazy validation for string elements within Thrift Python sets**, significantly **optimizing performance** by deferring validation until set elements are actually accessed. This **performance optimization** primarily impacts the **Thrift Python runtime**, specifically the `Set` class and its associated type information in `thrift/lib/python/types.pyx` and `thrift/lib/python/types.pxd`. It modifies numerous `Set` methods, such as `__hash__`, `__iter__`, and `__contains__`, to leverage this new lazy behavior, which allows string elements to be stored as `str` or `bytes` internally. The change includes a **test update** in `thrift/lib/python/test/serializer.py` to align with the new validation strategy.

This commit introduces **new memory benchmarks** to the **Thrift Python library**, specifically targeting the `immutable_map` implementation. The benchmarks, located in `thrift/lib/python/benchmark/struct_memory.py`, are designed to **detect memory leaks** during the **serialization and deserialization** of immutable maps. This **new capability** enhances the testing infrastructure, ensuring the `immutable_map` feature maintains efficient memory usage and prevents resource exhaustion in downstream applications. The added tests, including `immutable_map_leak`, will help proactively identify and prevent memory-related issues within the **Thrift Python runtime**.

This commit **adds a new memory benchmark** to the **Thrift Python library**, specifically targeting the `struct_memory.py` module. It introduces new functions, `immutable_string_set_impl` and `immutable_string_set`, to measure the memory footprint of **immutable string sets when embedded within Thrift structs**. This **new capability** provides crucial insights into the memory efficiency of these data structures, aiding in performance optimization efforts for Python Thrift applications.

This commit **enhances the Thrift Python benchmarking suite** by introducing **new performance tests** specifically for `map<i32, i32>` containers. It modifies `thrift/lib/python/benchmark/struct.thrift` to add an `int_map` field to `MyStruct`, enabling the benchmarking of map operations. The core **benchmark logic** is implemented in `thrift/lib/python/benchmark/containers.py`, covering initialization, random access, and iteration of these integer maps. This **new capability** provides more comprehensive performance metrics for map usage within the **Thrift Python runtime**, aiding in future performance analysis and optimization efforts.

This commit **adds warning messages** to the `validate` method within the **Thrift Python utility library** (`thrift/lib/py/util/TValidator.py`). This **maintenance** change aims to alert developers about the use of `hasattr(obj, thrift_spec)`, an internal attribute from `thrift-py-deprecated` that is incompatible with `thrift-python`. By flagging these deprecated usages, the commit helps ensure **forward compatibility** and prevents potential migration issues when transitioning to the newer `thrift-python` library. This proactively addresses a common problem identified during migration reviews.

This commit **enhances the performance benchmark suite** within the **`thrift/lib/python/benchmark`** module by introducing new tests for **Immutable map and set containers**. Specifically, it adds comprehensive benchmarks for **string-keyed maps**, covering initialization, random access, and iteration scenarios in `containers.py`. Additionally, existing integer-keyed map benchmarks are **renamed for improved clarity and consistency**. This **testing improvement** provides more granular performance insights into the `Immutable` data structures, particularly for common string-based key usage patterns, aiding in future optimizations of **Thrift Python container performance**.

This commit **fixes a compilation error** by **correcting the syntax** of `extern from` statements within the **Thrift Python types definition file** (`thrift/lib/python/types.pxd`). Specifically, it adds angle brackets (`<>`) around the module name in these statements to conform to the expected Cython syntax. This **syntax correction** resolves a build failure encountered by the `fbcode//admarket/ranking/automl/generative_model_building/midas/services/proposal_subgraph_matcher_service/tests:test_handler` target, ensuring proper **Cython compilation** for modules relying on these Thrift type definitions.