Skip to content

Commit

Permalink
Change FileInputStream Stats readTimeUs to readTimeNanos (facebookinc…
Browse files Browse the repository at this point in the history
…ubator#10822)

Summary:
Resolves: facebookincubator#10688

Pull Request resolved: facebookincubator#10822

Reviewed By: Yuhta

Differential Revision: D61839202

Pulled By: xiaoxmeng

fbshipit-source-id: 49f1b8fa5d93763912bbd02590c6c06183969f6d
  • Loading branch information
jinchengchenghh authored and facebook-github-bot committed Aug 27, 2024
1 parent 046726f commit 81a77f9
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 19 deletions.
20 changes: 10 additions & 10 deletions velox/common/file/FileInputStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ void FileInputStream::readNextRange() {
current_ = nullptr;

int32_t readBytes{0};
uint64_t readTimeUs{0};
uint64_t readTimeNs{0};
{
MicrosecondTimer timer{&readTimeUs};
NanosecondTimer timer{&readTimeNs};
if (readAheadWait_.valid()) {
readBytes = std::move(readAheadWait_)
.via(&folly::QueuedImmediateExecutor::instance())
Expand All @@ -72,7 +72,7 @@ void FileInputStream::readNextRange() {
readBytes = readSize();
VELOX_CHECK_LT(
0, readBytes, "Read past end of FileInputStream {}", fileSize_);
MicrosecondTimer timer{&readTimeUs};
NanosecondTimer timer{&readTimeNs};
file_->pread(fileOffset_, readBytes, buffer()->asMutable<char>());
}
}
Expand All @@ -82,7 +82,7 @@ void FileInputStream::readNextRange() {
current_ = ranges_.data();
fileOffset_ += readBytes;

updateStats(readBytes, readTimeUs);
updateStats(readBytes, readTimeNs);

maybeIssueReadahead();
}
Expand Down Expand Up @@ -222,9 +222,9 @@ void FileInputStream::maybeIssueReadahead() {
VELOX_CHECK(readAheadWait_.valid());
}

void FileInputStream::updateStats(uint64_t readBytes, uint64_t readTimeUs) {
void FileInputStream::updateStats(uint64_t readBytes, uint64_t readTimeNs) {
stats_.readBytes += readBytes;
stats_.readTimeUs += readTimeUs;
stats_.readTimeNs += readTimeNs;
++stats_.numReads;
}

Expand All @@ -243,15 +243,15 @@ FileInputStream::Stats FileInputStream::stats() const {

bool FileInputStream::Stats::operator==(
const FileInputStream::Stats& other) const {
return std::tie(numReads, readBytes, readTimeUs) ==
std::tie(other.numReads, other.readBytes, other.readTimeUs);
return std::tie(numReads, readBytes, readTimeNs) ==
std::tie(other.numReads, other.readBytes, other.readTimeNs);
}

std::string FileInputStream::Stats::toString() const {
return fmt::format(
"numReads: {}, readBytes: {}, readTimeUs: {}",
"numReads: {}, readBytes: {}, readTimeNs: {}",
numReads,
succinctBytes(readBytes),
succinctMicros(readTimeUs));
succinctMicros(readTimeNs));
}
} // namespace facebook::velox::common
4 changes: 2 additions & 2 deletions velox/common/file/FileInputStream.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class FileInputStream : public ByteInputStream {
struct Stats {
uint32_t numReads{0};
uint64_t readBytes{0};
uint64_t readTimeUs{0};
uint64_t readTimeNs{0};

bool operator==(const Stats& other) const;

Expand Down Expand Up @@ -106,7 +106,7 @@ class FileInputStream : public ByteInputStream {
return buffers_[nextBufferIndex()].get();
}

void updateStats(uint64_t readBytes, uint64_t readTimeUs);
void updateStats(uint64_t readBytes, uint64_t readTimeNs);

const std::unique_ptr<ReadFile> file_;
const uint64_t fileSize_;
Expand Down
4 changes: 2 additions & 2 deletions velox/common/file/tests/FileInputStreamTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ TEST_F(FileInputStreamTest, stats) {
ASSERT_EQ(
byteStream->stats().readBytes,
std::min(testData.streamSize, testData.bufferSize));
ASSERT_GE(byteStream->stats().readTimeUs, 0);
ASSERT_GT(byteStream->stats().readTimeNs, 0);
uint8_t buffer[testData.streamSize / 8];
for (int offset = 0; offset < testData.streamSize;) {
byteStream->readBytes(buffer, testData.streamSize / 8);
Expand All @@ -110,6 +110,6 @@ TEST_F(FileInputStreamTest, stats) {
bits::roundUp(testData.streamSize, testData.bufferSize) /
testData.bufferSize);
ASSERT_EQ(byteStream->stats().readBytes, testData.streamSize);
ASSERT_GE(byteStream->stats().readTimeUs, 0);
ASSERT_GT(byteStream->stats().readTimeNs, 0);
}
}
7 changes: 2 additions & 5 deletions velox/exec/SpillFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -328,13 +328,10 @@ void SpillReadFile::recordSpillStats() {
VELOX_CHECK(input_->atEnd());
const auto readStats = input_->stats();
common::updateGlobalSpillReadStats(
readStats.numReads,
readStats.readBytes,
readStats.readTimeUs * Timestamp::kNanosecondsInMicrosecond);
readStats.numReads, readStats.readBytes, readStats.readTimeNs);
auto lockedSpillStats = stats_->wlock();
lockedSpillStats->spillReads += readStats.numReads;
lockedSpillStats->spillReadTimeNanos +=
readStats.readTimeUs * Timestamp::kNanosecondsInMicrosecond;
lockedSpillStats->spillReadTimeNanos += readStats.readTimeNs;
lockedSpillStats->spillReadBytes += readStats.readBytes;
}
} // namespace facebook::velox::exec

0 comments on commit 81a77f9

Please sign in to comment.