Giuseppe Ottaviano

This commit **refactors** the **`folly::executors::FunctionScheduler`** by extracting the conditional variable wait calls from its main `run` loop into dedicated `waitForWork` methods. The `folly/executors/FunctionScheduler.cpp` implementation is updated to use these new methods, with corresponding declarations added to `folly/executors/FunctionScheduler.h`. This internal restructuring aims to provide **improved profiling capabilities** for the scheduler, allowing for more precise measurement of time spent waiting for work. The change enhances code clarity and diagnostic insights without altering the external behavior of the scheduler.

This commit introduces a **new GFlag** within the **`folly/executors`** library, specifically for the `CPUThreadPoolExecutor`, to allow for **disabling a dequeue optimization**. The `threadRun` function in `CPUThreadPoolExecutor.cpp` has been updated to conditionally apply this optimization based on the flag's state. This **maintenance** and **performance tuning** change provides operators with granular control, enabling them to bypass the optimization if it causes unexpected behavior or for specific testing scenarios. The added flexibility helps in diagnosing and mitigating potential performance or stability issues related to the executor's task dequeueing mechanism.

This commit introduces a **performance optimization** within the **`folly/executors`** library, specifically for `CPUThreadPoolExecutor` and `ThreadPoolExecutor`. It **prevents threads in non-dynamic thread pools from timing out** while waiting for new tasks, which was previously an unnecessary overhead since these threads cannot be stopped. By adding a `threadsCanTimeout_` flag to `ThreadPoolExecutor` and updating `setNumThreads` and `threadRun` in `CPUThreadPoolExecutor.cpp`, the system now avoids costly wake-ups. This change improves efficiency for services that utilize fixed-size thread pools, and a new test ensures the correct behavior.

This commit introduces a **new regression test** within the **`folly::logging`** subsystem, specifically for the `xlogIf` functionality. The test verifies that log message expressions are **conditionally evaluated**, ensuring they are not computed when logging is disabled. This is a crucial **maintenance** addition to safeguard an existing, subtle optimization that prevents potentially expensive message construction from impacting performance when logs are not active. It ensures that future changes do not inadvertently regress on this important performance characteristic of the logging framework.

This commit **refactors** the **`folly::executors` task queue implementations** and their usage within various **thread pool executors** to accept queue items as **rvalue references**. Specifically, methods like `add` and `addWithPriority` in `BlockingQueue.h` and other queue types now take `&&` arguments. This **performance optimization** allows for efficient move semantics, avoiding expensive copies for complex objects like `CPUTask` that contain `folly::Function`s. The change impacts `CPUThreadPoolExecutor`, `EDFThreadPoolExecutor`, `IOThreadPoolExecutor`, and `StripedEDFThreadPoolExecutor` by enabling them to leverage `std::move` or `folly::copy` when adding items, leading to more efficient resource handling.

This commit **refactors** the **`folly::CPUThreadPoolExecutor`** by changing the access specifier of the `addImpl` template method from `private` to `protected`. This modification, along with moving the method's definition into `folly/executors/CPUThreadPoolExecutor.h`, allows **derived classes** to access and potentially override this core task submission logic. The change enhances the **extensibility** of `CPUThreadPoolExecutor`, enabling more flexible custom executor implementations within the **`folly::executors`** library.

This commit introduces a **new feature**, the `StripedEDFThreadPoolExecutor`, to the **`folly/executors` library**. This alternative Earliest Deadline First (EDF) executor addresses scalability issues of the existing `EDFThreadPoolExecutor` by implementing a **striped priority queue** design, assigning one queue per LLC to reduce contention on large servers. It also utilizes a new intrusive heap protected by a `DistributedMutex` for its queue implementation. This change provides a more scalable solution for EDF task scheduling, particularly beneficial for high-core count systems, trading some prioritization accuracy for significantly improved performance.

This commit **refactors** the `addImpl()` method within the **`folly::CPUThreadPoolExecutor`** to accept a **generic enqueue task lambda**. This change simplifies the internal logic of `addImpl` by abstracting the task submission mechanism, which previously distinguished only between priority and non-priority enqueues. The primary impact is to **generalize task submission** for the CPU thread pool executor, enabling derived classes or future extensions to implement custom queuing capabilities beyond simple priority handling. This **enhancement** significantly improves the **extensibility** of the `CPUThreadPoolExecutor` by allowing arbitrary enqueue functions.

This commit performs a **refactoring** by **removing the `MutexWrapper` adapter** previously used by `fb303`'s `Synchronized` objects, which encapsulate `SharedMutex`. The wrapper was originally introduced to simplify the transition from `lock()` to `wlock()`. As part of this change, several call sites within the **`fboss/agent`** module, specifically in functions like `getTeFlowStats` and `getRouteCounterBytes`, are updated to explicitly use `wlock()` for write access to shared statistics pointers. This improves the clarity and correctness of the **locking mechanisms** for statistics, paving the way for future optimizations by allowing `rlock()` usage where read-only access is sufficient.

This commit performs a significant **refactoring** of the **`Hazptr` memory reclamation system** to eliminate its direct dependency on `folly::Executor`. This change addresses a potential **cyclic dependency** issue, particularly concerning `RequestContext`, by making `Hazptr` a more fundamental, low-dependency primitive. The internal scheduling mechanism for reclamation is updated to use a generic `hazptr_inline_executor_add` function, replacing direct `Executor` calls and allowing conditional scheduling. This improves the architectural purity and flexibility of `Hazptr`, simplifying inline fallback handling and preventing future dependency conflicts within core `folly` components.

This commit **refactors and optimizes** the `size()` method across several **`BlockingQueue` implementations** within `folly/executors/task_queue`. It improves efficiency by relying on the invariant that the semaphore's post count equals the number of queue entries, utilizing `sem_.valueGuess()` for a faster calculation. This change also **corrects a subtle bug** in bounded queue implementations related to this invariant and **removes the unused `sizeGuess()` method** for cleaner code. The affected classes include `LifoSemMPMCQueue`, `PriorityLifoSemMPMCQueue`, `PriorityUnboundedBlockingQueue`, and `UnboundedBlockingQueue`, resulting in more accurate and performant queue size retrieval.

This commit provides a **bug fix** and **refactoring** to ensure `RequestContext::saveContext()` can be safely called during thread-local object destruction, addressing a critical destruction order issue. It modifies the **`RequestContext`** subsystem, specifically `saveContext()`, `tryGetStaticContext()`, and `get()` in `folly/io/async/Request.cpp`, to gracefully handle calls made during thread shutdown. Concurrently, the underlying **`SingletonThreadLocal`** utility in `folly/SingletonThreadLocal.h` is refactored with a new `getLocalCache` function and checks to prevent assertion failures when thread-local storage is accessed during destruction. This change significantly enhances the stability and robustness of thread-local context management, preventing crashes in applications that interact with `RequestContext` during object teardown.

This commit performs a **refactoring** by removing the `SmallSerialExecutor` `typedef` from the codebase. This **maintenance** task streamlines internal type definitions, likely because the alias is no longer necessary or has been replaced by direct usage of the underlying type. The change primarily affects internal **concurrency management** or **task execution** components that previously relied on this specific `typedef`. This cleanup has no impact on external APIs or user-facing functionality, focusing solely on improving code clarity and reducing redundancy.

This commit performs a **refactoring** within the **Thrift C++2 server library**, specifically affecting the `ParallelConcurrencyController`. It updates the internal implementation of the `scheduleOnExecutor` function by replacing the use of `folly::SmallSerialExecutor` with `folly::SerialExecutor`. This **maintenance** change standardizes the executor type used for internal task scheduling, impacting the server's concurrency control mechanism without direct user-facing changes.

This commit **fixes** the `CancelTimeout` test case within the **`folly/io/async`** module's `TimerFDTimeoutManagerTest.cpp`. It **adjusts timeout durations** from microseconds to milliseconds and updates the corresponding assertions to reflect this change. This **test maintenance** ensures the test accurately validates the `TimerFDTimeoutManager`'s `cancelTimeout` functionality, improving the reliability of the asynchronous I/O timeout management tests. The change is isolated to the test suite and does not affect production code.

This commit **refactors** the handling of request contexts within **`folly::coro` collection and merge utilities** and **`folly::fibers` internal management**. It replaces manual context saving and restoring patterns with the more robust `RequestContextSaverScopeGuard` and `restoreContext()`. This change significantly improves **exception safety**, ensuring that request contexts are always properly restored even when exceptions are thrown within loops. Additionally, it optimizes performance by avoiding redundant `shared_ptr` copies and streamlines the codebase by removing direct calls to `RequestContext::setContext()`.

This commit **fixes a subtle bug** in the **`RequestContextSaverScopeGuard`** mechanism within `folly/io/async/Request.h` and `folly/io/async/Request.cpp`. Previously, when the request context changed between iterations, the old context's destruction could occur under the *new* context, potentially causing **unwanted propagation of request context** if destructors scheduled work. The change refactors the guard to explicitly unset and set contexts when they differ, introducing new `setContext()` and `restoreContext()` methods. This **improves the robustness and predictability** of request context management across various **executors** (`QueuedImmediateExecutor`, `SerialExecutor`, `StrandExecutor`) and `EventBase` components.

This commit **hardens the destruction of `RequestContext::StaticContext`** within the **`folly::io::async` module**. An explicit destructor for `StaticContext` is added in `folly/io/async/Request.cpp` to ensure the internal `requestContext` is properly reset during object destruction, preventing potential issues. Concurrently, the `StaticContext` class declaration in `folly/io/async/Request.h` is made more robust by explicitly deleting its copy and move constructors and assignment operators. This **maintenance** and **hardening** change improves the correctness and safety of `RequestContext` lifecycle management, mitigating subtle bugs related to object state and destruction.

This commit **fixes `RequestContext` leaks** across the **`folly::EventBase`** and its asynchronous execution paths, ensuring user callbacks are properly isolated and context is not inadvertently propagated. It achieves this by introducing `RequestContextSaverScopeGuard` to critical `EventBase` methods like `loopMain` and `runImmediatelyOrRunInEventBaseThread`, and by refactoring `folly::coro::Task` to use consistent context handling. A **new debug-only capability**, `DCheckRequestContextRestoredGuard`, is also added to verify proper `RequestContext` restoration, enhancing the **robustness and correctness** of Folly's asynchronous operations. These changes are validated by new tests and incur minimal overhead, as `RequestContext` is typically `nullptr` in the IO executor thread.

This commit **refactors** the `folly::SerialExecutor` within the **`folly::executors` library** to significantly improve its memory efficiency. It replaces the previous `UnboundedQueue` implementation, which was memory-intensive, with the more lightweight `SerialExecutorMPSCQueue`. This change effectively promotes the `SmallSerialExecutor`'s efficient design to become the default `SerialExecutor`, making it suitable for fine-grained use cases. Consequently, all existing uses of `SerialExecutor` will now automatically benefit from a **more memory-efficient and performant executor**, while `SmallSerialExecutor` becomes an alias to this improved implementation.