Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ target_include_directories(eloqstore PRIVATE ${LIBZMQ_INCLUDE_DIRS})
# PUBLIC: engine public headers (async_io_manager.h, kill_point.h) include
# glog/logging.h.
target_link_libraries(eloqstore PUBLIC glog::glog)
target_link_libraries(eloqstore PRIVATE ${URING_LIB} absl::flat_hash_map ${BOOST_CONTEXT_TARGET} ${CURL_LIBRARIES} jsoncpp_lib ${ZSTD_LIBRARY} aws-cpp-sdk-core OpenSSL::Crypto ${LIBZMQ_LIBRARIES})
target_link_libraries(eloqstore PRIVATE ${URING_LIB} absl::base absl::flat_hash_map ${BOOST_CONTEXT_TARGET} ${CURL_LIBRARIES} jsoncpp_lib ${ZSTD_LIBRARY} aws-cpp-sdk-core OpenSSL::Crypto ${LIBZMQ_LIBRARIES})

set(ELOQ_STORE_CAPI_INCLUDE
${ELOQ_STORE_INCLUDE}
Expand Down
82 changes: 10 additions & 72 deletions src/storage/shard.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <vector>

#if defined(__x86_64__) || defined(_M_X64)
#include <absl/base/internal/sysinfo.h>
#include <x86intrin.h> // For __rdtsc()
#endif

Expand Down Expand Up @@ -1422,85 +1423,22 @@ bool Shard::HasPendingRequests() const
return requests_.size_approx() > 0;
}

/** @brief
* Measure TSC frequency by sleeping for 1ms and measuring cycles.
* Retries until stable (within 1% difference) or up to 16ms total.
* Should be called once during data substrate initialization.
* This function is thread-safe and will only execute once.
*/
/** @brief Initialize the hardware-counter frequency once. */
void Shard::InitializeTscFrequency()
{
#if defined(__x86_64__) || defined(_M_X64)
std::call_once(
tsc_frequency_initialized_,
[]()
{
constexpr uint64_t SLEEP_MICROSECONDS = 1000; // 1ms
constexpr uint64_t MAX_TOTAL_MICROSECONDS = 16000; // 16ms max
constexpr double STABILITY_THRESHOLD =
0.01; // 1% difference for stability

uint64_t prev_freq = 0;
uint64_t total_slept = 0;
int stable_count = 0;
constexpr int REQUIRED_STABLE_COUNT =
2; // Need 2 consecutive stable measurements

while (total_slept < MAX_TOTAL_MICROSECONDS)
{
uint64_t start_cycles = __rdtsc();
std::this_thread::sleep_for(
std::chrono::microseconds(SLEEP_MICROSECONDS));
uint64_t end_cycles = __rdtsc();
uint64_t elapsed_cycles = end_cycles - start_cycles;
uint64_t freq = elapsed_cycles /
SLEEP_MICROSECONDS; // cycles per microsecond

total_slept += SLEEP_MICROSECONDS;

// Check if frequency is stable (within 1% of previous
// measurement)
if (prev_freq > 0)
{
double diff_ratio =
(freq > prev_freq)
? static_cast<double>(freq - prev_freq) / prev_freq
: static_cast<double>(prev_freq - freq) / prev_freq;
if (diff_ratio <= STABILITY_THRESHOLD)
{
stable_count++;
if (stable_count >= REQUIRED_STABLE_COUNT)
{
// Frequency is stable, use the average
tsc_cycles_per_microsecond_.store(
(prev_freq + freq) / 2,
std::memory_order_release);
return;
}
}
else
{
stable_count = 0; // Reset stability counter
}
}

prev_freq = freq;
}

// If we couldn't get stable measurement, use the last measured
// value
if (prev_freq > 0)
{
tsc_cycles_per_microsecond_.store(prev_freq,
std::memory_order_release);
}
else
{
// Fallback to approximate value if measurement failed
tsc_cycles_per_microsecond_.store(2000,
std::memory_order_release);
}
}); // End of lambda passed to std::call_once
// This frequency uses the same raw TSC scale as __rdtsc().
const double frequency_hz =
absl::base_internal::NominalCPUFrequency();
const uint64_t cycles_per_microsecond = std::max<uint64_t>(
1, static_cast<uint64_t>(frequency_hz / 1'000'000.0));
tsc_cycles_per_microsecond_.store(cycles_per_microsecond,
std::memory_order_release);
Comment on lines +1435 to +1440

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the repository-selected Abseil implementation and its fallback paths.
fd -HI '^sysinfo\.(cc|h)$' . | while IFS= read -r file; do
  rg -n -C 12 'NominalCPUFrequency|GetNominalCPUFrequency|return 1\.0|tsc_freq_khz' "$file"
done

# Identify the selected Abseil version or source pin.
rg -n -C 3 'absl|ABSL|FetchContent|GIT_TAG' -g 'CMakeLists.txt' -g '*.cmake' .

Repository: eloqdata/eloqstore

Length of output: 19988


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== candidate Abseil declarations/versions in cmake and deps=="
rg -n -C 8 'abseil|ABSL|absl|FetchContent_Declare\(|absl-base|absl_core|base_internal|sysinfo' cmake CMakeLists.txt rust/eloqstore-sys/vendor/CMakeLists.txt || true

echo
echo "== local sysinfo files =="
fd -HI 'sysinfo\.(cc|h)$' . -t f | sort

echo
echo "== repository-wide selected abseil sources =="
git ls-files | rg 'abseil|absl.*sysinfo|absl/base/internal/sysinfo' || true

echo
echo "== src/storage/shard.cpp header and TSC-related context =="
wc -l src/storage/shard.cpp
sed -n '1,40p;1360,1470p' src/storage/shard.cpp

echo
echo "== all NominalCPUFrequency references =="
rg -n -C 5 'NominalCPUFrequency|absl::base_internal' .

Repository: eloqdata/eloqstore

Length of output: 29588


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== external/abseil candidate version/source markers =="
for f in external/abseil/CMakeLists.txt external/abseil/git_rev external/abseil/source external/abseil/version external/abseil/src/version external/abseil/README.md external/abseil/src/absl/base/internal/sysinfo.cc external/abseil/absl/base/internal/sysinfo.cc; do
  if [ -e "$f" ]; then
    echo "--- $f"
    sed -n '1,120p' "$f"
  fi
done

echo
echo "== external/abseil sysinfo implementation =="
if [ -f external/abseil/absl/base/internal/sysinfo.cc ]; then
  FILE=external/abseil/absl/base/internal/sysinfo.cc
else
  FILE=external/abseil/src/absl/base/internal/sysinfo.cc
fi
wc -l "$FILE"
rg -n -C 20 'GetNominalCPUFrequency|return_freq|NominalCPUFrequency|return 1\.0|1\.0|tsc_freq_khz|cpufreq|ReadFile|sysinfo' "$FILE"

echo
echo "== local TSC consumer context =="
wc -l src/storage/shard.cpp
sed -n '1470,1520p' src/storage/shard.cpp
rg -n -C 4 'ReadTimeMicroseconds|InitializeTscFrequency|capped_spin|sleep_for|work-loop|budget|deadline' src/storage src -g '*.cpp' -g '*.h' | head -n 240

Repository: eloqdata/eloqstore

Length of output: 338


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== tracked Abseil/source paths =="
git ls-files | rg '(^|/)abseil/|absl/base/internal/sysinfo\.(cc|h)$|CMakeLists\.txt$' | sed -n '1,220p'

echo
echo "== local filesystem directories/subtrees =="
python3 - <<'PY'
import os
for root in ['external', 'src', 'rust/eloqstore-sys', '.']:
    if not os.path.isdir(root):
        continue
    print(f"-- {root}")
    for entry in sorted(os.listdir(root)[:200]):
        path=os.path.join(root, entry)
        print(entry, 'DIR' if os.path.isdir(path) else os.path.getsize(path) if os.path.isfile(path) else '?')
PY

echo
echo "== candidate sysinfo paths under tracked files =="
git ls-files 'external/**' 'src/**' 'rust/**' 'cmake/**' 'CMakeLists.txt' \
| rg 'sysinfo|abseil|absl|CMakeLists.txt' \
| sort \
| rg 'sysinfo' || true

echo
echo "== explicit Abseil versions if present =="
rg -n -C 4 'GIT_TAG|GIT_REPOSITORY|GIT_SUBMODULE|abseil|ABSL' .gitmodules CMakeLists.txt cmake/external cmake dependencies.cmake external/** rust/eloqstore-sys/** -E 'abseil|absl|GIT_TAG|absl' 2>/dev/null | sed -n '1,260p' || true

Repository: eloqdata/eloqstore

Length of output: 1614


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== local external/abseil directory listing (top levels) =="
find external/abseil -maxdepth 2 -printf '%p %Y\n' 2>/dev/null | sort | sed -n '1,220p'

echo
echo "== local sysinfo source candidates =="
while IFS= read -r file; do
  echo "--- $file"
  wc -l "$file"
  rg -n -C 20 'GetNominalCPUFrequency|NominalCPUFrequency|return_freq|return 1\.0|1\.0|tsc_freq_khz|cpufreq|ReadFile|sysinfo' "$file" || true
done < <(find external/abseil -type f -name 'sysinfo.cc' -print 2>/dev/null | sort)

echo
echo "== version markers and submodule hints =="
if [ -f .gitmodules ]; then
  cat .gitmodules
fi
for file in VERSION README.md cmake/dependencies.cmake CMakeLists.txt; do
  if [ -f "$file" ]; then
    echo "--- $file"
    rg -n -C 4 'abseil|GIT_TAG|GIT_REPOSITORY|version|external/abseil' "$file" || true
  fi
done

Repository: eloqdata/eloqstore

Length of output: 325


🌐 Web query:

Abseil NominalCPUFrequency return 1.0 source absl/base/internal/sysinfo GetNominalCPUFrequency fallback

💡 Result:

In the Abseil C++ library, the function absl::base_internal::GetNominalCPUFrequency (and the public-facing NominalCPUFrequency which calls it) returns 1.0 as a fallback value when it cannot successfully determine the CPU frequency through platform-specific APIs [1][2][3]. The value 1.0 serves as a safe, non-zero default to prevent issues like division-by-zero that could occur if the library attempted to use a 0.0 frequency in performance calculations [2][3]. The implementation of GetNominalCPUFrequency uses a series of platform-dependent checks to detect the CPU frequency [1][4][3]. If these checks fail—such as when a system call returns an error, registry keys are inaccessible (e.g., in UWP applications), or a file cannot be read from /sys/devices/system/cpu—the function executes a fallback return statement of 1.0 [1][2][5][3]. Common scenarios where this fallback is triggered include: - UWP (Universal Windows Platform) applications, where access to the Windows registry is restricted [5][3]. - Systems where the expected platform-specific APIs (such as sysctl on certain BSD or macOS configurations) fail to provide the frequency data [6]. - Environments where the necessary system files (e.g., /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_max_freq) are missing or inaccessible [1][7]. Because NominalCPUFrequency is designed to be called before main and before memory allocation is fully initialized, it is implemented to be lightweight and must not allocate memory, which is why it relies on these direct, low-level platform inquiries rather than complex configuration lookups [1][8][3].

Citations:


Reject the NominalCPUFrequency() fallback.

Nil/unavailable platform frequency lookup makes absl::base_internal::NominalCPUFrequency() return 1.0. The std::max<uint64_t>(1, ...) clamp then makes ReadTimeMicroseconds() advance 1 million times faster than real time. This causes work-loop budgets to run too long and delayed-request deadlines to expire early. Use a verified calibrated fallback or fall back to monotonic time when the reported frequency is not plausible.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/storage/shard.cpp` around lines 1439 - 1444, Update the initialization
around tsc_cycles_per_microsecond_ so it does not use
absl::base_internal::NominalCPUFrequency() when the reported frequency is
unavailable or implausibly low. Use a verified calibrated cycles-per-microsecond
value, or make ReadTimeMicroseconds() fall back to monotonic time, while
preserving the existing release store for valid frequency measurements.

});
#elif defined(__aarch64__)
std::call_once(tsc_frequency_initialized_,
[]()
Expand Down
Loading