Skip to content

Commit 7c49b9c

Browse files
authored
[TieredStorage] Use BufWriter in TieredWritableFile (pyth-network#261)
#### Problem TieredWritableFile currently uses File instead of BufWriter. This will introduce more syscall when doing file writes. #### Summary of Changes This PR makes TieredWritableFile uses BufWriter to allow the write-call to be more optimized to reduce the number of syscalls. #### Test Plan Existing tiered-storage test. Will run experiments to verify its performance improvement. #### Dependency anza-xyz/agave#260
1 parent 69b40a4 commit 7c49b9c

File tree

6 files changed

+60
-58
lines changed

6 files changed

+60
-58
lines changed

accounts-db/src/tiered_storage.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ impl TieredStorage {
125125

126126
if format == &HOT_FORMAT {
127127
let result = {
128-
let writer = HotStorageWriter::new(&self.path)?;
128+
let mut writer = HotStorageWriter::new(&self.path)?;
129129
writer.write_accounts(accounts, skip)
130130
};
131131

accounts-db/src/tiered_storage/file.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use {
22
bytemuck::{AnyBitPattern, NoUninit},
33
std::{
44
fs::{File, OpenOptions},
5-
io::{Read, Result as IoResult, Seek, SeekFrom, Write},
5+
io::{BufWriter, Read, Result as IoResult, Seek, SeekFrom, Write},
66
mem,
77
path::Path,
88
},
@@ -77,22 +77,22 @@ impl TieredReadableFile {
7777
}
7878

7979
#[derive(Debug)]
80-
pub struct TieredWritableFile(pub File);
80+
pub struct TieredWritableFile(pub BufWriter<File>);
8181

8282
impl TieredWritableFile {
8383
pub fn new(file_path: impl AsRef<Path>) -> IoResult<Self> {
84-
Ok(Self(
84+
Ok(Self(BufWriter::new(
8585
OpenOptions::new()
8686
.create_new(true)
8787
.write(true)
8888
.open(file_path)?,
89-
))
89+
)))
9090
}
9191

9292
/// Writes `value` to the file.
9393
///
9494
/// `value` must be plain ol' data.
95-
pub fn write_pod<T: NoUninit>(&self, value: &T) -> IoResult<usize> {
95+
pub fn write_pod<T: NoUninit>(&mut self, value: &T) -> IoResult<usize> {
9696
// SAFETY: Since T is NoUninit, it does not contain any uninitialized bytes.
9797
unsafe { self.write_type(value) }
9898
}
@@ -107,22 +107,22 @@ impl TieredWritableFile {
107107
/// Caller must ensure casting T to bytes is safe.
108108
/// Refer to the Safety sections in std::slice::from_raw_parts()
109109
/// and bytemuck's Pod and NoUninit for more information.
110-
pub unsafe fn write_type<T>(&self, value: &T) -> IoResult<usize> {
110+
pub unsafe fn write_type<T>(&mut self, value: &T) -> IoResult<usize> {
111111
let ptr = value as *const _ as *const u8;
112112
let bytes = unsafe { std::slice::from_raw_parts(ptr, mem::size_of::<T>()) };
113113
self.write_bytes(bytes)
114114
}
115115

116-
pub fn seek(&self, offset: u64) -> IoResult<u64> {
117-
(&self.0).seek(SeekFrom::Start(offset))
116+
pub fn seek(&mut self, offset: u64) -> IoResult<u64> {
117+
self.0.seek(SeekFrom::Start(offset))
118118
}
119119

120-
pub fn seek_from_end(&self, offset: i64) -> IoResult<u64> {
121-
(&self.0).seek(SeekFrom::End(offset))
120+
pub fn seek_from_end(&mut self, offset: i64) -> IoResult<u64> {
121+
self.0.seek(SeekFrom::End(offset))
122122
}
123123

124-
pub fn write_bytes(&self, bytes: &[u8]) -> IoResult<usize> {
125-
(&self.0).write_all(bytes)?;
124+
pub fn write_bytes(&mut self, bytes: &[u8]) -> IoResult<usize> {
125+
self.0.write_all(bytes)?;
126126

127127
Ok(bytes.len())
128128
}

accounts-db/src/tiered_storage/footer.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ impl TieredStorageFooter {
190190
Self::new_from_footer_block(&file)
191191
}
192192

193-
pub fn write_footer_block(&self, file: &TieredWritableFile) -> TieredStorageResult<()> {
193+
pub fn write_footer_block(&self, file: &mut TieredWritableFile) -> TieredStorageResult<()> {
194194
// SAFETY: The footer does not contain any uninitialized bytes.
195195
unsafe { file.write_type(self)? };
196196
file.write_pod(&TieredStorageMagicNumber::default())?;
@@ -356,8 +356,8 @@ mod tests {
356356

357357
// Persist the expected footer.
358358
{
359-
let file = TieredWritableFile::new(&path.path).unwrap();
360-
expected_footer.write_footer_block(&file).unwrap();
359+
let mut file = TieredWritableFile::new(&path.path).unwrap();
360+
expected_footer.write_footer_block(&mut file).unwrap();
361361
}
362362

363363
// Reopen the same storage, and expect the persisted footer is

accounts-db/src/tiered_storage/hot.rs

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,7 @@ impl HotStorageReader {
542542
}
543543

544544
fn write_optional_fields(
545-
file: &TieredWritableFile,
545+
file: &mut TieredWritableFile,
546546
opt_fields: &AccountMetaOptionalFields,
547547
) -> TieredStorageResult<usize> {
548548
let mut size = 0;
@@ -572,7 +572,7 @@ impl HotStorageWriter {
572572
/// Persists an account with the specified information and returns
573573
/// the stored size of the account.
574574
fn write_account(
575-
&self,
575+
&mut self,
576576
lamports: u64,
577577
owner_offset: OwnerOffset,
578578
account_data: &[u8],
@@ -599,7 +599,7 @@ impl HotStorageWriter {
599599
stored_size += self
600600
.storage
601601
.write_bytes(&PADDING_BUFFER[0..(padding_len as usize)])?;
602-
stored_size += write_optional_fields(&self.storage, &optional_fields)?;
602+
stored_size += write_optional_fields(&mut self.storage, &optional_fields)?;
603603

604604
Ok(stored_size)
605605
}
@@ -614,7 +614,7 @@ impl HotStorageWriter {
614614
U: StorableAccounts<'a, T>,
615615
V: Borrow<AccountHash>,
616616
>(
617-
&self,
617+
&mut self,
618618
accounts: &StorableAccountsWithHashesAndWriteVersions<'a, 'b, T, U, V>,
619619
skip: usize,
620620
) -> TieredStorageResult<Vec<StoredAccountInfo>> {
@@ -677,7 +677,7 @@ impl HotStorageWriter {
677677
footer.index_block_offset = cursor as u64;
678678
cursor += footer
679679
.index_block_format
680-
.write_index_block(&self.storage, &index)?;
680+
.write_index_block(&mut self.storage, &index)?;
681681
if cursor % HOT_BLOCK_ALIGNMENT != 0 {
682682
// In case it is not yet aligned, it is due to the fact that
683683
// the index block has an odd number of entries. In such case,
@@ -692,9 +692,9 @@ impl HotStorageWriter {
692692
footer.owner_count = owners_table.len() as u32;
693693
footer
694694
.owners_block_format
695-
.write_owners_block(&self.storage, &owners_table)?;
695+
.write_owners_block(&mut self.storage, &owners_table)?;
696696

697-
footer.write_footer_block(&self.storage)?;
697+
footer.write_footer_block(&mut self.storage)?;
698698

699699
Ok(stored_infos)
700700
}
@@ -892,8 +892,8 @@ pub mod tests {
892892
};
893893

894894
{
895-
let file = TieredWritableFile::new(&path).unwrap();
896-
expected_footer.write_footer_block(&file).unwrap();
895+
let mut file = TieredWritableFile::new(&path).unwrap();
896+
expected_footer.write_footer_block(&mut file).unwrap();
897897
}
898898

899899
// Reopen the same storage, and expect the persisted footer is
@@ -928,7 +928,7 @@ pub mod tests {
928928
..TieredStorageFooter::default()
929929
};
930930
{
931-
let file = TieredWritableFile::new(&path).unwrap();
931+
let mut file = TieredWritableFile::new(&path).unwrap();
932932
let mut current_offset = 0;
933933

934934
account_offsets = hot_account_metas
@@ -942,7 +942,7 @@ pub mod tests {
942942
// while the test only focuses on account metas, writing a footer
943943
// here is necessary to make it a valid tiered-storage file.
944944
footer.index_block_offset = current_offset as u64;
945-
footer.write_footer_block(&file).unwrap();
945+
footer.write_footer_block(&mut file).unwrap();
946946
}
947947

948948
let hot_storage = HotStorageReader::new_from_path(&path).unwrap();
@@ -971,8 +971,8 @@ pub mod tests {
971971
};
972972

973973
{
974-
let file = TieredWritableFile::new(&path).unwrap();
975-
footer.write_footer_block(&file).unwrap();
974+
let mut file = TieredWritableFile::new(&path).unwrap();
975+
footer.write_footer_block(&mut file).unwrap();
976976
}
977977

978978
let hot_storage = HotStorageReader::new_from_path(&path).unwrap();
@@ -1016,14 +1016,14 @@ pub mod tests {
10161016
..TieredStorageFooter::default()
10171017
};
10181018
{
1019-
let file = TieredWritableFile::new(&path).unwrap();
1019+
let mut file = TieredWritableFile::new(&path).unwrap();
10201020

10211021
let cursor = footer
10221022
.index_block_format
1023-
.write_index_block(&file, &index_writer_entries)
1023+
.write_index_block(&mut file, &index_writer_entries)
10241024
.unwrap();
10251025
footer.owners_block_offset = cursor as u64;
1026-
footer.write_footer_block(&file).unwrap();
1026+
footer.write_footer_block(&mut file).unwrap();
10271027
}
10281028

10291029
let hot_storage = HotStorageReader::new_from_path(&path).unwrap();
@@ -1059,20 +1059,20 @@ pub mod tests {
10591059
};
10601060

10611061
{
1062-
let file = TieredWritableFile::new(&path).unwrap();
1062+
let mut file = TieredWritableFile::new(&path).unwrap();
10631063

10641064
let mut owners_table = OwnersTable::default();
10651065
addresses.iter().for_each(|owner_address| {
10661066
owners_table.insert(owner_address);
10671067
});
10681068
footer
10691069
.owners_block_format
1070-
.write_owners_block(&file, &owners_table)
1070+
.write_owners_block(&mut file, &owners_table)
10711071
.unwrap();
10721072

10731073
// while the test only focuses on account metas, writing a footer
10741074
// here is necessary to make it a valid tiered-storage file.
1075-
footer.write_footer_block(&file).unwrap();
1075+
footer.write_footer_block(&mut file).unwrap();
10761076
}
10771077

10781078
let hot_storage = HotStorageReader::new_from_path(&path).unwrap();
@@ -1118,7 +1118,7 @@ pub mod tests {
11181118
let account_offsets: Vec<_>;
11191119

11201120
{
1121-
let file = TieredWritableFile::new(&path).unwrap();
1121+
let mut file = TieredWritableFile::new(&path).unwrap();
11221122
let mut current_offset = 0;
11231123

11241124
account_offsets = hot_account_metas
@@ -1141,12 +1141,12 @@ pub mod tests {
11411141
});
11421142
footer
11431143
.owners_block_format
1144-
.write_owners_block(&file, &owners_table)
1144+
.write_owners_block(&mut file, &owners_table)
11451145
.unwrap();
11461146

11471147
// while the test only focuses on account metas, writing a footer
11481148
// here is necessary to make it a valid tiered-storage file.
1149-
footer.write_footer_block(&file).unwrap();
1149+
footer.write_footer_block(&mut file).unwrap();
11501150
}
11511151

11521152
let hot_storage = HotStorageReader::new_from_path(&path).unwrap();
@@ -1237,7 +1237,7 @@ pub mod tests {
12371237
};
12381238

12391239
{
1240-
let file = TieredWritableFile::new(&path).unwrap();
1240+
let mut file = TieredWritableFile::new(&path).unwrap();
12411241
let mut current_offset = 0;
12421242

12431243
// write accounts blocks
@@ -1264,7 +1264,7 @@ pub mod tests {
12641264
footer.index_block_offset = current_offset as u64;
12651265
current_offset += footer
12661266
.index_block_format
1267-
.write_index_block(&file, &index_writer_entries)
1267+
.write_index_block(&mut file, &index_writer_entries)
12681268
.unwrap();
12691269

12701270
// write owners block
@@ -1275,10 +1275,10 @@ pub mod tests {
12751275
});
12761276
footer
12771277
.owners_block_format
1278-
.write_owners_block(&file, &owners_table)
1278+
.write_owners_block(&mut file, &owners_table)
12791279
.unwrap();
12801280

1281-
footer.write_footer_block(&file).unwrap();
1281+
footer.write_footer_block(&mut file).unwrap();
12821282
}
12831283

12841284
let hot_storage = HotStorageReader::new_from_path(&path).unwrap();
@@ -1358,7 +1358,7 @@ pub mod tests {
13581358
let temp_dir = TempDir::new().unwrap();
13591359
let path = temp_dir.path().join("test_write_account_and_index_blocks");
13601360
let stored_infos = {
1361-
let writer = HotStorageWriter::new(&path).unwrap();
1361+
let mut writer = HotStorageWriter::new(&path).unwrap();
13621362
writer.write_accounts(&storable_accounts, 0).unwrap()
13631363
};
13641364

accounts-db/src/tiered_storage/index.rs

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ impl IndexBlockFormat {
5959
/// the total number of bytes written.
6060
pub fn write_index_block(
6161
&self,
62-
file: &TieredWritableFile,
62+
file: &mut TieredWritableFile,
6363
index_entries: &[AccountIndexWriterEntry<impl AccountOffset>],
6464
) -> TieredStorageResult<usize> {
6565
match self {
@@ -181,9 +181,11 @@ mod tests {
181181
.collect();
182182

183183
{
184-
let file = TieredWritableFile::new(&path).unwrap();
184+
let mut file = TieredWritableFile::new(&path).unwrap();
185185
let indexer = IndexBlockFormat::AddressesThenOffsets;
186-
let cursor = indexer.write_index_block(&file, &index_entries).unwrap();
186+
let cursor = indexer
187+
.write_index_block(&mut file, &index_entries)
188+
.unwrap();
187189
footer.owners_block_offset = cursor as u64;
188190
}
189191

@@ -223,8 +225,8 @@ mod tests {
223225
{
224226
// we only write a footer here as the test should hit an assert
225227
// failure before it actually reads the file.
226-
let file = TieredWritableFile::new(&path).unwrap();
227-
footer.write_footer_block(&file).unwrap();
228+
let mut file = TieredWritableFile::new(&path).unwrap();
229+
footer.write_footer_block(&mut file).unwrap();
228230
}
229231

230232
let file = OpenOptions::new()
@@ -259,8 +261,8 @@ mod tests {
259261
{
260262
// we only write a footer here as the test should hit an assert
261263
// failure before it actually reads the file.
262-
let file = TieredWritableFile::new(&path).unwrap();
263-
footer.write_footer_block(&file).unwrap();
264+
let mut file = TieredWritableFile::new(&path).unwrap();
265+
footer.write_footer_block(&mut file).unwrap();
264266
}
265267

266268
let file = OpenOptions::new()
@@ -294,8 +296,8 @@ mod tests {
294296
{
295297
// we only write a footer here as the test should hit an assert
296298
// failure before we actually read the file.
297-
let file = TieredWritableFile::new(&path).unwrap();
298-
footer.write_footer_block(&file).unwrap();
299+
let mut file = TieredWritableFile::new(&path).unwrap();
300+
footer.write_footer_block(&mut file).unwrap();
299301
}
300302

301303
let file = OpenOptions::new()
@@ -334,8 +336,8 @@ mod tests {
334336
{
335337
// we only write a footer here as the test should hit an assert
336338
// failure before we actually read the file.
337-
let file = TieredWritableFile::new(&path).unwrap();
338-
footer.write_footer_block(&file).unwrap();
339+
let mut file = TieredWritableFile::new(&path).unwrap();
340+
footer.write_footer_block(&mut file).unwrap();
339341
}
340342

341343
let file = OpenOptions::new()

accounts-db/src/tiered_storage/owners.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ impl OwnersBlockFormat {
4747
/// Persists the provided owners' addresses into the specified file.
4848
pub fn write_owners_block(
4949
&self,
50-
file: &TieredWritableFile,
50+
file: &mut TieredWritableFile,
5151
owners_table: &OwnersTable,
5252
) -> TieredStorageResult<usize> {
5353
match self {
@@ -139,20 +139,20 @@ mod tests {
139139
};
140140

141141
{
142-
let file = TieredWritableFile::new(&path).unwrap();
142+
let mut file = TieredWritableFile::new(&path).unwrap();
143143

144144
let mut owners_table = OwnersTable::default();
145145
addresses.iter().for_each(|owner_address| {
146146
owners_table.insert(owner_address);
147147
});
148148
footer
149149
.owners_block_format
150-
.write_owners_block(&file, &owners_table)
150+
.write_owners_block(&mut file, &owners_table)
151151
.unwrap();
152152

153153
// while the test only focuses on account metas, writing a footer
154154
// here is necessary to make it a valid tiered-storage file.
155-
footer.write_footer_block(&file).unwrap();
155+
footer.write_footer_block(&mut file).unwrap();
156156
}
157157

158158
let file = OpenOptions::new().read(true).open(path).unwrap();

0 commit comments

Comments
 (0)