Skip to content

Commit

Permalink
Increased the fiber stack size to 64KB
Browse files Browse the repository at this point in the history
Summary:
During the release testing (https://fburl.com/conveyor/a16yhrk4), tasks were crashing mostly by
SIGSEGV and SIGABRT. From the further analysis, it is suspected that the destructor calls from
the fiber run by the Navy's region_manager thread are causing random memory corruptions. In order to
fix this issue, this change increases the stack size of fibers used by the NavyThread to 64KB. The
increases in the stack memory usage would not be an issue for region_manager for now because the
number of fibers are bound by the number of clean regions, so the stack overhead will be bound by
the preallocated guard page stacks (100 by default); i.e., 64KB x 100 = 64MB.

Reviewed By: therealgymmy

Differential Revision: D50756161

fbshipit-source-id: 9c110504f51a2822e108571dbbf57f5d7aecb98b
  • Loading branch information
Jaesoo Lee authored and facebook-github-bot committed Oct 28, 2023
1 parent 1bbb9ad commit c9d7902
Showing 1 changed file with 16 additions and 2 deletions.
18 changes: 16 additions & 2 deletions cachelib/navy/common/NavyThread.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,26 @@ namespace navy {
*/
class NavyThread {
public:
struct Options {
static constexpr size_t kDefaultStackSize{64 * 1024};
constexpr Options() {}

/**
* Maximum stack size for fibers which will be used for executing all the
* tasks.
*/
size_t stackSize{kDefaultStackSize};
};

/**
* Initializes with current EventBaseManager and passed-in thread name.
*/
explicit NavyThread(folly::StringPiece name) {
explicit NavyThread(folly::StringPiece name, Options options = Options()) {
th_ = std::make_unique<folly::ScopedEventBaseThread>(name.str());
fm_ = &folly::fibers::getFiberManager(*th_->getEventBase());

folly::fibers::FiberManager::Options opts;
opts.stackSize = options.stackSize;
fm_ = &folly::fibers::getFiberManager(*th_->getEventBase(), opts);
}

~NavyThread() { th_.reset(); }
Expand Down

0 comments on commit c9d7902

Please sign in to comment.