Skip to content

Commit

Permalink
Remove unused-but-set variables in velox/common/caching/tests/SsdFile…
Browse files Browse the repository at this point in the history
…Test.cpp +16 (facebookincubator#9828)

Summary:
Pull Request resolved: facebookincubator#9828

This diff removes a variable that was set, but which was not used.

LLVM-15 has a warning `-Wunused-but-set-variable` which we treat as an error because it's so often diagnostic of a code issue. Unused but set variables often indicate a programming mistake, but can also just be unnecessary cruft that harms readability and performance.

Removing this variable will not change how your code works, but the unused variable may indicate your code isn't working the way you thought it was. If you feel the diff needs changes before landing, **please commandeer** and make appropriate changes: there are hundreds of these and responding to them individually is challenging.

For questions/comments, contact r-barnes.

 - If you approve of this diff, please use the "Accept & Ship" button :-)

Reviewed By: palmje, Yuhta

Differential Revision: D57336836

fbshipit-source-id: ee3af6aa3f893c423ae8f97a01c415df78b2e2a5
  • Loading branch information
r-barnes authored and facebook-github-bot committed May 23, 2024
1 parent e790c80 commit 179fefc
Show file tree
Hide file tree
Showing 15 changed files with 2 additions and 42 deletions.
4 changes: 1 addition & 3 deletions velox/common/memory/MallocAllocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,8 @@ bool MallocAllocator::allocateContiguousImpl(
} else {
VELOX_CHECK_LE(numPages, maxPages);
}
[[maybe_unused]] MachinePageCount numCollateralPages = 0;
if (collateral != nullptr) {
numCollateralPages =
freeNonContiguous(*collateral) / AllocationTraits::kPageSize;
freeNonContiguous(*collateral);
}
auto numContiguousCollateralPages = allocation.numPages();
if (numContiguousCollateralPages > 0) {
Expand Down
2 changes: 0 additions & 2 deletions velox/common/memory/tests/ByteStreamTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,10 @@ TEST_F(ByteStreamTest, inputStream) {
uint8_t* const kFakeBuffer = reinterpret_cast<uint8_t*>(this);
std::vector<ByteRange> byteRanges;
size_t totalBytes{0};
[[maybe_unused]] size_t lastRangeEnd;
for (int32_t i = 0; i < 32; ++i) {
byteRanges.push_back(ByteRange{kFakeBuffer, 4096 + i, 0});
totalBytes += 4096 + i;
}
lastRangeEnd = byteRanges.back().size;
ByteInputStream byteStream(std::move(byteRanges));
ASSERT_EQ(byteStream.size(), totalBytes);
}
Expand Down
3 changes: 0 additions & 3 deletions velox/dwio/common/tests/utils/BatchMaker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,11 @@ VectorPtr createScalar(
BufferPtr nulls = allocateNulls(size, &pool);
auto* nullsPtr = nulls->asMutable<uint64_t>();

size_t nullCount = 0;
for (size_t i = 0; i < size; ++i) {
auto notNull = isNotNull(gen, i, isNullAt);
bits::setNull(nullsPtr, i, !notNull);
if (notNull) {
valuesPtr[i] = val();
} else {
nullCount++;
}
}

Expand Down
2 changes: 0 additions & 2 deletions velox/dwio/common/tests/utils/MapBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ class MapBuilder {

BufferPtr valueNulls = allocateNulls(items, &pool);
auto* valueNullsPtr = valueNulls->asMutable<uint64_t>();
size_t valueNullCount = 0;

auto i = 0;
auto offset = 0;
Expand All @@ -74,7 +73,6 @@ class MapBuilder {
valuesPtr[offset] = *pair.second;
bits::clearNull(valueNullsPtr, offset);
} else {
valueNullCount++;
bits::setNull(valueNullsPtr, offset);
}
++offset;
Expand Down
4 changes: 0 additions & 4 deletions velox/dwio/dwrf/test/ReaderTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1926,7 +1926,6 @@ void testFlatmapAsMapFieldLifeCycle(
auto child =
std::dynamic_pointer_cast<MapVector>(result->as<RowVector>()->childAt(0));
BaseVector* rowPtr = result.get();
MapVector* childPtr = child.get();
Buffer* rawNulls = child->nulls().get();
BufferPtr sizes = child->sizes();
Buffer* rawOffsets = child->offsets().get();
Expand All @@ -1947,7 +1946,6 @@ void testFlatmapAsMapFieldLifeCycle(

auto mapKeys = child->mapKeys();
auto rawSizes = child->sizes().get();
childPtr = child.get();
child.reset();

EXPECT_TRUE(rowReader->next(batchSize, result));
Expand All @@ -1959,7 +1957,6 @@ void testFlatmapAsMapFieldLifeCycle(
EXPECT_NE(mapKeys, child->mapKeys());
// there is a TODO in FlatMapColumnReader next() (result is not reused)
// should be EQ; fix: https://fburl.com/code/wtrq8r5q
// EXPECT_EQ(childPtr, child.get());
EXPECT_EQ(rowPtr, result.get());

EXPECT_TRUE(rowReader->next(batchSize, result));
Expand All @@ -1971,7 +1968,6 @@ void testFlatmapAsMapFieldLifeCycle(
EXPECT_NE(rawSizes, childCurr->sizes().get());
EXPECT_NE(rawOffsets, childCurr->offsets().get());
EXPECT_NE(keysPtr, childCurr->mapKeys().get());
// EXPECT_EQ(childPtr, childCurr.get());
EXPECT_EQ(rowPtr, result.get());
}

Expand Down
4 changes: 0 additions & 4 deletions velox/dwio/dwrf/test/TestDecompression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1155,8 +1155,6 @@ TEST_F(TestSeek, uncompressedLarge) {
return 0;
};
// Start and size of last Next as offsets to content (no headers).
int32_t lastReadStart = 0;
int32_t lastReadSize = 0;

for (auto& pair : ranges) {
uint64_t target = pair.first;
Expand All @@ -1170,8 +1168,6 @@ TEST_F(TestSeek, uncompressedLarge) {
EXPECT_EQ(result, addressForOffset(target + readSize));
EXPECT_EQ(
size, readSizeForAddress(reinterpret_cast<const char*>(result)));
lastReadStart = target + readSize;
lastReadSize = size;
readSize += size;
} while (readSize < targetSize);
}
Expand Down
2 changes: 0 additions & 2 deletions velox/dwio/dwrf/test/TestStatisticsBuilderUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -280,10 +280,8 @@ TEST_F(TestStatisticsBuilderUtils, addBinaryValues) {

std::array<char, 10> data;
std::memset(data.data(), 'a', 10);
size_t total = 0;
for (size_t i = 0; i < size; ++i) {
valuesPtr[i] = StringView(data.data(), i + 1);
total += (i + 1);
}

{
Expand Down
2 changes: 0 additions & 2 deletions velox/dwio/dwrf/test/TestStripeStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ void enqueueReads(
auto& metadataCache = readerBase.getMetadataCache();
uint64_t offset = stripeStart;
uint64_t length = 0;
uint32_t regions = 0;
for (const auto& stream : footer.streams()) {
length = stream.length();
// If index cache is available, there is no need to read it
Expand All @@ -81,7 +80,6 @@ void enqueueReads(
selector.shouldReadStream(stream.node(), stream.sequence()) &&
!inMetaCache) {
input.enqueue({offset, length});
regions++;
}
offset += length;
}
Expand Down
4 changes: 0 additions & 4 deletions velox/dwio/parquet/reader/ParquetTypeWithId.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,12 +103,8 @@ LevelMode ParquetTypeWithId::makeLevelInfo(LevelInfo& info) const {
bool isMap = type()->kind() == TypeKind::MAP;
bool hasList = false;
if (isStruct) {
bool isAllLists = true;
for (auto i = 0; i < getChildren().size(); ++i) {
auto& child = parquetChildAt(i);
if (child.type()->kind() != TypeKind ::ARRAY) {
isAllLists = false;
}
hasList |= hasList || containsList(child);
}
}
Expand Down
2 changes: 0 additions & 2 deletions velox/exec/benchmarks/FilterProjectBenchmark.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -253,9 +253,7 @@ class FilterProjectBenchmark : public VectorTestBase {

int64_t run(std::shared_ptr<const core::PlanNode> plan) {
auto start = getCurrentTimeMicro();
int32_t numRows = 0;
auto result = exec::test::AssertQueryBuilder(plan).copyResults(pool_.get());
numRows += result->childAt(0)->as<FlatVector<int64_t>>()->valueAt(0);
auto elapsedMicros = getCurrentTimeMicro() - start;
return elapsedMicros;
}
Expand Down
2 changes: 0 additions & 2 deletions velox/exec/benchmarks/HashTableBenchmark.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,6 @@ class HashTableBenchmark : public VectorTestBase {
int32_t mask = powerOfTwo - 1;
int32_t position = 0;
int32_t delta = 1;
int32_t numInserted = 0;
auto nextOffset = rowContainer->nextOffset();

// We insert values in a geometric skip order. 1, 2, 4, 7,
Expand All @@ -340,7 +339,6 @@ class HashTableBenchmark : public VectorTestBase {
if (nextOffset) {
*reinterpret_cast<char**>(newRow + nextOffset) = nullptr;
}
++numInserted;
for (auto i = 0; i < batches[batchIndex]->type()->size(); ++i) {
rowContainer->store(decoded[batchIndex][i], rowIndex, newRow, i);
}
Expand Down
2 changes: 0 additions & 2 deletions velox/exec/fuzzer/PrestoQueryRunner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -611,12 +611,10 @@ std::vector<RowVectorPtr> PrestoQueryRunner::execute(const std::string& sql) {
auto response = ServerResponse(startQuery(sql));
response.throwIfFailed();

vector_size_t numResults = 0;
std::vector<RowVectorPtr> queryResults;
for (;;) {
for (auto& result : response.queryResults(pool_.get())) {
queryResults.push_back(result);
numResults += result->size();
}

if (response.queryCompleted()) {
Expand Down
6 changes: 0 additions & 6 deletions velox/exec/tests/HashTableTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -452,16 +452,12 @@ class HashTableTest : public testing::TestWithParam<bool>,
const auto mode = topTable_->hashMode();
SelectivityInfo hashTime;
SelectivityInfo probeTime;
int32_t numHashed = 0;
int32_t numProbed = 0;
int32_t numHit = 0;
auto& hashers = topTable_->hashers();
VectorHasher::ScratchMemory scratchMemory;
for (auto batchIndex = 0; batchIndex < batches_.size(); ++batchIndex) {
const auto& batch = batches_[batchIndex];
lookup->reset(batch->size());
rows.setAll();
numHashed += batch->size();
{
SelectivityTimer timer(hashTime, 0);
for (auto i = 0; i < hashers.size(); ++i) {
Expand Down Expand Up @@ -496,13 +492,11 @@ class HashTableTest : public testing::TestWithParam<bool>,
}
} else {
{
numProbed += lookup->rows.size();
SelectivityTimer timer(probeTime, 0);
topTable_->joinProbe(*lookup);
}
for (auto i = 0; i < lookup->rows.size(); ++i) {
const auto key = lookup->rows[i];
numHit += lookup->hits[key] != nullptr;
ASSERT_EQ(rowOfKey_[startOffset + key], lookup->hits[key]);
}
}
Expand Down
2 changes: 0 additions & 2 deletions velox/experimental/wave/exec/tests/utils/FileFormat.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -250,10 +250,8 @@ StringView StringSet::add(StringView data) {

std::unique_ptr<Column> StringSet::toColumn() {
auto buffer = AlignedBuffer::allocate<char>(totalSize_, pool_);
int64_t fill = 0;
for (auto& piece : buffers_) {
memcpy(buffer->asMutable<char>(), piece->as<char>(), piece->size());
fill += piece->size();
}
auto column = std::make_unique<Column>();
column->kind = TypeKind::VARCHAR;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,8 @@ class TwoStringKeysBenchmark : public HiveConnectorTestBase {

auto task = makeTask(plan);

vector_size_t numResultRows = 0;
while (auto result = task->next()) {
numResultRows += result->size();
// no action
}

LOG(ERROR) << exec::printPlanWithStats(
Expand Down

0 comments on commit 179fefc

Please sign in to comment.