From e00fd45bba98dcc22b14e3555684245a43bee160 Mon Sep 17 00:00:00 2001 From: Erik Grinaker Date: Tue, 3 Jun 2025 14:20:34 +0200 Subject: [PATCH] page_api: remove smallvec (#12095) ## Problem The gRPC `page_api` domain types used smallvecs to avoid heap allocations in the common case where a single page is requested. However, this is pointless: the Protobuf types use a normal vec, and converting a smallvec into a vec always causes a heap allocation anyway. ## Summary of changes Use a normal `Vec` instead of a `SmallVec` in `page_api` domain types. --- Cargo.lock | 1 - pageserver/page_api/Cargo.toml | 1 - pageserver/page_api/src/model.rs | 13 ++++++------- pageserver/src/page_service.rs | 4 ++-- 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9fc233e5ec..542a4528d3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4469,7 +4469,6 @@ dependencies = [ "pageserver_api", "postgres_ffi", "prost 0.13.5", - "smallvec", "thiserror 1.0.69", "tonic 0.13.1", "tonic-build", diff --git a/pageserver/page_api/Cargo.toml b/pageserver/page_api/Cargo.toml index 4f62c77eb2..e643b5749b 100644 --- a/pageserver/page_api/Cargo.toml +++ b/pageserver/page_api/Cargo.toml @@ -9,7 +9,6 @@ bytes.workspace = true pageserver_api.workspace = true postgres_ffi.workspace = true prost.workspace = true -smallvec.workspace = true thiserror.workspace = true tonic.workspace = true utils.workspace = true diff --git a/pageserver/page_api/src/model.rs b/pageserver/page_api/src/model.rs index 0268ab920b..a6853895d9 100644 --- a/pageserver/page_api/src/model.rs +++ b/pageserver/page_api/src/model.rs @@ -14,7 +14,6 @@ use std::fmt::Display; use bytes::Bytes; use postgres_ffi::Oid; -use smallvec::SmallVec; // TODO: split out Lsn, RelTag, SlruKind, Oid and other basic types to a separate crate, to avoid // pulling in all of their other crate dependencies when building the client. use utils::lsn::Lsn; @@ -302,7 +301,7 @@ pub struct GetPageRequest { /// Multiple pages will be executed as a single batch by the Pageserver, amortizing layer access /// costs and parallelizing them. This may increase the latency of any individual request, but /// improves the overall latency and throughput of the batch as a whole. - pub block_numbers: SmallVec<[u32; 1]>, + pub block_numbers: Vec, } impl TryFrom for GetPageRequest { @@ -320,7 +319,7 @@ impl TryFrom for GetPageRequest { .ok_or(ProtocolError::Missing("read_lsn"))? .try_into()?, rel: pb.rel.ok_or(ProtocolError::Missing("rel"))?.try_into()?, - block_numbers: pb.block_number.into(), + block_numbers: pb.block_number, }) } } @@ -337,7 +336,7 @@ impl TryFrom for proto::GetPageRequest { request_class: request.request_class.into(), read_lsn: Some(request.read_lsn.try_into()?), rel: Some(request.rel.into()), - block_number: request.block_numbers.into_vec(), + block_number: request.block_numbers, }) } } @@ -410,7 +409,7 @@ pub struct GetPageResponse { /// A string describing the status, if any. pub reason: Option, /// The 8KB page images, in the same order as the request. Empty if status != OK. - pub page_images: SmallVec<[Bytes; 1]>, + pub page_images: Vec, } impl From for GetPageResponse { @@ -419,7 +418,7 @@ impl From for GetPageResponse { request_id: pb.request_id, status_code: pb.status_code.into(), reason: Some(pb.reason).filter(|r| !r.is_empty()), - page_images: pb.page_image.into(), + page_images: pb.page_image, } } } @@ -430,7 +429,7 @@ impl From for proto::GetPageResponse { request_id: response.request_id, status_code: response.status_code.into(), reason: response.reason.unwrap_or_default(), - page_image: response.page_images.into_vec(), + page_image: response.page_images, } } } diff --git a/pageserver/src/page_service.rs b/pageserver/src/page_service.rs index b9ba4a3555..735c944970 100644 --- a/pageserver/src/page_service.rs +++ b/pageserver/src/page_service.rs @@ -765,7 +765,7 @@ impl PageStreamError { request_id, status_code, reason: Some(status.message().to_string()), - page_images: SmallVec::new(), + page_images: Vec::new(), } .into()) } @@ -3521,7 +3521,7 @@ impl GrpcPageServiceHandler { request_id: req.request_id, status_code: page_api::GetPageStatusCode::Ok, reason: None, - page_images: SmallVec::with_capacity(results.len()), + page_images: Vec::with_capacity(results.len()), }; for result in results {