Skip to content

Commit

Permalink
Use Microseconds for decoding time (facebookincubator#8589)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: facebookincubator#8589

The decoding times are in the single digits ms most of the times, even 1 ms is normal. So let's give it more resolution than 1 ms.

Reviewed By: helfman

Differential Revision: D53200548

fbshipit-source-id: 8154a80e4a4163c51c3efece792d64df3048fb03
  • Loading branch information
Daniel Munoz authored and facebook-github-bot committed Feb 4, 2024
1 parent c30fa9d commit 735b8f4
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 12 deletions.
10 changes: 5 additions & 5 deletions velox/dwio/common/Options.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ class RowReaderOptions {
// (in dwrf row reader). todo: encapsulate this and keySelectionCallBack_ in a
// struct
std::function<void(uint64_t)> blockedOnIoCallback_;
std::function<void(uint64_t)> decodingTimeMsCallback_;
std::function<void(uint64_t)> decodingTimeUsCallback_;
std::function<void(uint16_t)> stripeCountCallback_;
bool eagerFirstStripeLoad = true;
uint64_t skipRows_ = 0;
Expand Down Expand Up @@ -350,12 +350,12 @@ class RowReaderOptions {
return blockedOnIoCallback_;
}

void setDecodingTimeMsCallback(std::function<void(int64_t)> decodingTimeMs) {
decodingTimeMsCallback_ = std::move(decodingTimeMs);
void setDecodingTimeUsCallback(std::function<void(int64_t)> decodingTimeUs) {
decodingTimeUsCallback_ = std::move(decodingTimeUs);
}

std::function<void(int64_t)> getDecodingTimeMsCallback() const {
return decodingTimeMsCallback_;
std::function<void(int64_t)> getDecodingTimeUsCallback() const {
return decodingTimeUsCallback_;
}

void setStripeCountCallback(
Expand Down
21 changes: 15 additions & 6 deletions velox/dwio/dwrf/reader/DwrfReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
*/

#include "velox/dwio/dwrf/reader/DwrfReader.h"

#include <chrono>

#include "velox/dwio/common/TypeUtils.h"
#include "velox/dwio/common/exception/Exception.h"
#include "velox/dwio/dwrf/reader/ColumnReader.h"
Expand All @@ -35,7 +38,7 @@ DwrfRowReader::DwrfRowReader(
: StripeReaderBase(reader),
options_(opts),
executor_{options_.getDecodingExecutor()},
decodingTimeMsCallback_{options_.getDecodingTimeMsCallback()},
decodingTimeUsCallback_{options_.getDecodingTimeUsCallback()},
stripeCountCallback_{options_.getStripeCountCallback()},
columnSelector_{std::make_shared<ColumnSelector>(
ColumnSelector::apply(opts.getSelector(), reader->getSchema()))} {
Expand Down Expand Up @@ -274,17 +277,23 @@ void DwrfRowReader::readNext(
const dwio::common::Mutation* mutation,
VectorPtr& result) {
if (!selectiveColumnReader_) {
const auto startTime = std::chrono::high_resolution_clock::now();
std::optional<std::chrono::steady_clock::time_point> startTime;
if (decodingTimeUsCallback_) {
// We'll use wall time since we have parallel decoding.
// If we move to sequential decoding only, we can use CPU time.
startTime.emplace(std::chrono::steady_clock::now());
}
// TODO: Move row number appending logic here. Currently this is done in
// the wrapper reader.
VELOX_CHECK(
mutation == nullptr,
"Mutation pushdown is only supported in selective reader");
columnReader_->next(rowsToRead, result);
if (decodingTimeMsCallback_) {
auto decodingTime = std::chrono::duration_cast<std::chrono::milliseconds>(
std::chrono::high_resolution_clock::now() - startTime);
decodingTimeMsCallback_(decodingTime.count());
if (startTime.has_value()) {
decodingTimeUsCallback_(
std::chrono::duration_cast<std::chrono::microseconds>(
std::chrono::steady_clock::now() - startTime.value())
.count());
}
return;
}
Expand Down
2 changes: 1 addition & 1 deletion velox/dwio/dwrf/reader/DwrfReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ class DwrfRowReader : public StrideIndexProvider,
std::shared_ptr<StripeDictionaryCache> stripeDictionaryCache_;
dwio::common::RowReaderOptions options_;
std::shared_ptr<folly::Executor> executor_;
std::function<void(uint64_t)> decodingTimeMsCallback_;
std::function<void(uint64_t)> decodingTimeUsCallback_;
std::function<void(uint16_t)> stripeCountCallback_;

struct PrefetchedStripeState {
Expand Down

0 comments on commit 735b8f4

Please sign in to comment.