Skip to content

Commit f7a6543

Browse files
authored
Merge pull request #37160 from vespa-engine/toregge/improve-transient-memory-for-flush-calculation
Improve transient_memory_for_flush calculation.
2 parents 0e4e287 + e124122 commit f7a6543

20 files changed

Lines changed: 88 additions & 40 deletions

searchcore/src/tests/proton/documentmetastore/documentmetastore_test.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -598,7 +598,8 @@ TEST(DocumentMetaStoreTest, gids_can_be_saved_and_loaded) {
598598
uint64_t expSaveBytesSize =
599599
DocumentMetaStore::minHeaderLen + (1000 - 4) * DocumentMetaStore::entry_size(true, false);
600600
EXPECT_EQ(expSaveBytesSize, dms1.getEstimatedSaveByteSize());
601-
EXPECT_EQ(0, dms1.transient_memory_for_flush());
601+
EXPECT_EQ(0, dms1.transient_memory_for_flush(false));
602+
EXPECT_EQ(expSaveBytesSize, dms1.transient_memory_for_flush(true));
602603
TuneFileAttributes tuneFileAttributes;
603604
DummyFileHeaderContext fileHeaderContext;
604605
AttributeFileSaveTarget saveTarget(tuneFileAttributes, fileHeaderContext);

searchcore/src/vespa/searchcore/proton/attribute/flushableattribute.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -245,8 +245,7 @@ double FlushableAttribute::get_replay_operation_cost() const {
245245
}
246246

247247
size_t FlushableAttribute::transient_memory_for_flush() const noexcept {
248-
size_t flush_buffer_size = _hwInfo.disk().slow() ? _attr->getEstimatedSaveByteSize() : 0;
249-
return _attr->transient_memory_for_flush() + flush_buffer_size;
248+
return _attr->transient_memory_for_flush(_hwInfo.disk().slow());
250249
}
251250

252251
std::chrono::steady_clock::duration FlushableAttribute::last_flush_duration() const noexcept {

searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1146,8 +1146,9 @@ uint64_t DocumentMetaStore::getEstimatedSaveByteSize() const {
11461146
return estimate;
11471147
}
11481148

1149-
size_t DocumentMetaStore::transient_memory_for_flush() const noexcept {
1150-
return 0;
1149+
size_t DocumentMetaStore::transient_memory_for_flush(bool slow_disk) const noexcept {
1150+
size_t flush_buffer_size = slow_disk ? getEstimatedSaveByteSize() : 0;
1151+
return flush_buffer_size;
11511152
}
11521153

11531154
uint32_t DocumentMetaStore::getVersion() const {

searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -306,7 +306,7 @@ class DocumentMetaStore final : public DocumentMetaStoreAttribute,
306306
void onShrinkLidSpace() override;
307307
size_t getEstimatedShrinkLidSpaceGain() const override;
308308
uint64_t getEstimatedSaveByteSize() const override;
309-
[[nodiscard]] size_t transient_memory_for_flush() const noexcept override;
309+
[[nodiscard]] size_t transient_memory_for_flush(bool slow_disk) const noexcept override;
310310
uint32_t getVersion() const override;
311311

312312
/*

searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastoreflushtarget.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,7 @@ uint64_t DocumentMetaStoreFlushTarget::getApproxBytesToWriteToDisk() const {
225225
}
226226

227227
size_t DocumentMetaStoreFlushTarget::transient_memory_for_flush() const noexcept {
228-
size_t flush_buffer_size = _hwInfo.disk().slow() ? _dms->getEstimatedSaveByteSize() : 0;
229-
return flush_buffer_size + _dms->transient_memory_for_flush();
228+
return _dms->transient_memory_for_flush(_hwInfo.disk().slow());
230229
}
231230

232231
std::chrono::steady_clock::duration DocumentMetaStoreFlushTarget::last_flush_duration() const noexcept {

searchlib/src/tests/attribute/array_bool_attribute/array_bool_attribute_test.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -538,7 +538,8 @@ TEST_F(ArrayBoolAttributeTest, estimated_save_byte_size) {
538538
// 4096 + (8+7)/8 + 6*5 = 4096 + 1 + 30 = 4127 (with 1 reserved doc, 5 added = 6 docs)
539539
uint64_t expected = 4096 + (8 + 7) / 8 + 6 * 5;
540540
EXPECT_EQ(expected, estimate);
541-
EXPECT_EQ(6 * sizeof(vespalib::datastore::EntryRef), _attr->transient_memory_for_flush());
541+
EXPECT_EQ(6 * sizeof(vespalib::datastore::EntryRef), _attr->transient_memory_for_flush(false));
542+
EXPECT_EQ(6 * sizeof(vespalib::datastore::EntryRef) + estimate, _attr->transient_memory_for_flush(true));
542543
}
543544

544545
class ArrayBoolExtAttributeTest : public ::testing::Test {

searchlib/src/tests/attribute/attribute_test.cpp

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,26 @@ bool preciseEstimatedSize(const AttributeVector& a) {
139139
return true;
140140
}
141141

142+
bool single_normal_numeric_attribute(const AttributeVector& a) {
143+
if (a.getCollectionType() != search::attribute::CollectionType::SINGLE) {
144+
return false;
145+
}
146+
if (a.getConfig().fastSearch()) {
147+
return false;
148+
}
149+
switch (a.getBasicType()) {
150+
case BasicType::INT8:
151+
case BasicType::INT16:
152+
case BasicType::INT32:
153+
case BasicType::INT64:
154+
case BasicType::DOUBLE:
155+
case BasicType::FLOAT:
156+
return true;
157+
default:
158+
return false;
159+
}
160+
}
161+
142162
string baseFileName(const string& attrName) {
143163
return tmpDir + "/" + attrName;
144164
}
@@ -472,7 +492,20 @@ template <typename VectorType, typename BufferType> void AttributeTest::testRelo
472492
exp_transient_memory_for_flush =
473493
BitVector::legacy_num_bytes_with_single_guard_bit(a->getCommittedDocIdLimit());
474494
}
475-
EXPECT_EQ(exp_transient_memory_for_flush, a->transient_memory_for_flush());
495+
if (a->getBasicType() == BasicType::UINT2 && !multivalue_attr) {
496+
exp_transient_memory_for_flush = 4 * ((a->getCommittedDocIdLimit() + 15) / 16);
497+
}
498+
if (a->getBasicType() == BasicType::UINT4 && !multivalue_attr) {
499+
exp_transient_memory_for_flush = 4 * ((a->getCommittedDocIdLimit() + 7) / 8);
500+
}
501+
EXPECT_EQ(exp_transient_memory_for_flush, a->transient_memory_for_flush(false));
502+
if (single_normal_numeric_attribute(*a)) {
503+
// Buffer handover from attribute saver to attribute memory file writer
504+
EXPECT_EQ((size_t)a->getEstimatedSaveByteSize(), a->transient_memory_for_flush(true));
505+
} else {
506+
EXPECT_EQ((size_t)a->getEstimatedSaveByteSize() + exp_transient_memory_for_flush,
507+
a->transient_memory_for_flush(true));
508+
}
476509
double actSize = statSize(*b);
477510
EXPECT_LE(actSize, a->size_on_disk());
478511
EXPECT_EQ(stat_size_on_disk(*b), a->size_on_disk());

searchlib/src/tests/attribute/predicate_attribute/predicate_attribute_test.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,8 @@ TEST_F(PredicateAttributeTest, save_and_load_predicate_attribute) {
123123
auto attr = make_sample_predicate_attribute();
124124
std::filesystem::path file_name(tmp_dir);
125125
file_name.append(attr_name);
126-
EXPECT_NE(0, attr->transient_memory_for_flush());
126+
EXPECT_NE(0, attr->transient_memory_for_flush(false));
127+
EXPECT_NE(0, attr->transient_memory_for_flush(true));
127128
attr->save(file_name.native());
128129
EXPECT_NE(0, attr->size_on_disk());
129130
auto attr2 = make_attribute(file_name.native(), attr->getConfig(), false);
@@ -132,7 +133,8 @@ TEST_F(PredicateAttributeTest, save_and_load_predicate_attribute) {
132133
EXPECT_TRUE(attr2->isLoaded());
133134
EXPECT_EQ(11, attr2->getCommittedDocIdLimit());
134135
EXPECT_EQ(attr->size_on_disk(), attr2->size_on_disk());
135-
EXPECT_EQ(attr->transient_memory_for_flush(), attr2->transient_memory_for_flush());
136+
EXPECT_EQ(attr->transient_memory_for_flush(false), attr2->transient_memory_for_flush(false));
137+
EXPECT_EQ(attr->transient_memory_for_flush(true), attr2->transient_memory_for_flush(true));
136138
}
137139

138140
TEST_F(PredicateAttributeTest, buffer_size_mismatch_is_fatal_during_load) {

searchlib/src/tests/attribute/raw_attribute/raw_attribute_test.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,8 @@ TEST_F(RawAttributeTest, save_and_load) {
232232
EXPECT_EQ(0, _attr->size_on_disk());
233233
EXPECT_EQ(zero_flush_duration, _attr->last_flush_duration());
234234
auto estimated_write_bytes = _attr->getEstimatedSaveByteSize();
235-
EXPECT_EQ(11 * sizeof(EntryRef), _attr->transient_memory_for_flush());
235+
EXPECT_EQ(11 * sizeof(EntryRef), _attr->transient_memory_for_flush(false));
236+
EXPECT_EQ(11 * sizeof(EntryRef) + estimated_write_bytes, _attr->transient_memory_for_flush(true));
236237
_attr->save();
237238
EXPECT_EQ(std::filesystem::file_size(attr_path), estimated_write_bytes);
238239
auto saved_size_on_disk = _attr->size_on_disk();

searchlib/src/tests/attribute/tensorattribute/tensorattribute_test.cpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1698,18 +1698,24 @@ TEST_F(SparseTensorAttributeTest, size_on_disk_factor_is_calculated_and_used) {
16981698
}
16991699
EXPECT_LT(10, attr.getCommittedDocIdLimit());
17001700
EXPECT_THAT(attr.getEstimatedSaveByteSize(), AllOf(Ge(40_Ki), Le(50_Ki)));
1701-
EXPECT_EQ(attr.getCommittedDocIdLimit() * sizeof(EntryRef), attr.transient_memory_for_flush());
1701+
EXPECT_EQ(attr.getCommittedDocIdLimit() * sizeof(EntryRef), attr.transient_memory_for_flush(false));
1702+
EXPECT_EQ(attr.getCommittedDocIdLimit() * sizeof(EntryRef) + attr.getEstimatedSaveByteSize(),
1703+
attr.transient_memory_for_flush(true));
17021704
attr.save((_test_dir / "tensor").string());
17031705
auto size_on_disk = attr.size_on_disk();
17041706
EXPECT_LT(60_Ki, size_on_disk);
17051707
EXPECT_THAT(attr.getEstimatedSaveByteSize(), AllOf(Ge(size_on_disk - 4_Ki), Le(size_on_disk + 4_Ki)));
1706-
EXPECT_EQ(attr.getCommittedDocIdLimit() * sizeof(EntryRef), attr.transient_memory_for_flush());
1708+
EXPECT_EQ(attr.getCommittedDocIdLimit() * sizeof(EntryRef), attr.transient_memory_for_flush(false));
1709+
EXPECT_EQ(attr.getCommittedDocIdLimit() * sizeof(EntryRef) + attr.getEstimatedSaveByteSize(),
1710+
attr.transient_memory_for_flush(true));
17071711
auto real_attr2 = std::make_shared<SerializedFastValueAttribute>((_test_dir / "tensor").string(), cfg);
17081712
AttributeVector& attr2 = *real_attr2;
17091713
ASSERT_TRUE(attr2.load());
17101714
EXPECT_EQ(size_on_disk, attr2.size_on_disk());
17111715
EXPECT_THAT(attr2.getEstimatedSaveByteSize(), AllOf(Ge(size_on_disk - 4_Ki), Le(size_on_disk + 4_Ki)));
1712-
EXPECT_EQ(attr2.getCommittedDocIdLimit() * sizeof(EntryRef), attr2.transient_memory_for_flush());
1716+
EXPECT_EQ(attr2.getCommittedDocIdLimit() * sizeof(EntryRef), attr2.transient_memory_for_flush(false));
1717+
EXPECT_EQ(attr2.getCommittedDocIdLimit() * sizeof(EntryRef) + attr2.getEstimatedSaveByteSize(),
1718+
attr2.transient_memory_for_flush(true));
17131719
EXPECT_EQ(dynamic_memory_usage, attr2.getStatus().get_used_minus_dead_and_onhold() - initial_memory_usage);
17141720
EXPECT_EQ(attr.getEstimatedSaveByteSize(), attr2.getEstimatedSaveByteSize());
17151721
}

0 commit comments

Comments
 (0)