Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Implement the Buf to avoid extra memory allocation #4090

Closed
wants to merge 2 commits into from
Closed

feat: Implement the Buf to avoid extra memory allocation #4090

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3c0b73e
Implement the Buf to avoid extra memory allocation #4065
LYZJU2019 Jun 2, 2024
198eebf
Merge branch 'main' into impl-buf
tisonkun Jun 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions src/mito2/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ common-time.workspace = true
common-wal.workspace = true
crc32fast = "1"
crossbeam-utils.workspace = true
datafusion.workspace = true
datafusion-common.workspace = true
datafusion-expr.workspace = true
datafusion.workspace = true
datatypes.workspace = true
dotenv.workspace = true
futures.workspace = true
Expand All @@ -47,6 +47,7 @@ log-store = { workspace = true, optional = true }
memcomparable = "0.2"
moka = { workspace = true, features = ["sync", "future"] }
object-store.workspace = true
opendal = { version = "*" }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's re-export the Buffer via object-store crate

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should it be store-api crate instead of object-store crate?

Copy link
Member

@WenyXu WenyXu Jun 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about letting the into_bytes return an enum and importing the object-store in the mito crate?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LYZJU2019 Hi, thank you for your contribution. Could you refactor it as @WenyXu said? Let's merge this PR after the refactoring.

parquet = { workspace = true, features = ["async"] }
paste.workspace = true
pin-project.workspace = true
Expand All @@ -67,9 +68,9 @@ snafu.workspace = true
store-api.workspace = true
strum.workspace = true
table.workspace = true
tokio.workspace = true
tokio-stream.workspace = true
tokio-util.workspace = true
tokio.workspace = true
uuid.workspace = true

[dev-dependencies]
Expand Down
7 changes: 4 additions & 3 deletions src/mito2/src/wal/entry_reader.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use api::v1::WalEntry;
use async_stream::stream;
use futures::StreamExt;
use opendal::Buffer;
use prost::Message;
use snafu::{ensure, ResultExt};
use store_api::logstore::entry::Entry;
Expand All @@ -28,9 +29,9 @@ pub(crate) fn decode_raw_entry(raw_entry: Entry) -> Result<(EntryId, WalEntry)>
let entry_id = raw_entry.entry_id();
let region_id = raw_entry.region_id();
ensure!(raw_entry.is_complete(), CorruptedEntrySnafu { region_id });
// TODO(weny): implement the [Buf] for return value, avoid extra memory allocation.
let bytes = raw_entry.into_bytes();
let wal_entry = WalEntry::decode(bytes.as_slice()).context(DecodeWalSnafu { region_id })?;
// TODO(yuliu): implement the [Buf] for return value, avoid extra memory allocation.
let buffer: Buffer = raw_entry.into_bytes().into();
let wal_entry = WalEntry::decode(buffer).context(DecodeWalSnafu { region_id })?;

Ok((entry_id, wal_entry))
}
Expand Down
Loading