Skip to content

Make bridge::Buffer generic again. #97539

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.

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
50 changes: 26 additions & 24 deletions library/proc_macro/src/bridge/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,38 +5,40 @@ use std::mem;
use std::ops::{Deref, DerefMut};
use std::slice;

// `T` is always `u8` in practice. Attempts to remove `T` caused mild
// performance regressions, surprisingly enough.
Comment on lines +8 to +9
Copy link
Member

Choose a reason for hiding this comment

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

Actually, every fn in this module might need #[inline] (all 17 of them), so can you just edit the comment to say that without generics, #[inline] would be needed everywhere to avoid performance regressions and r=me?

#[repr(C)]
pub struct Buffer {
data: *mut u8,
pub struct Buffer<T: Copy> {
data: *mut T,
len: usize,
capacity: usize,
reserve: extern "C" fn(Buffer, usize) -> Buffer,
drop: extern "C" fn(Buffer),
reserve: extern "C" fn(Buffer<T>, usize) -> Buffer<T>,
drop: extern "C" fn(Buffer<T>),
}

unsafe impl Sync for Buffer {}
unsafe impl Send for Buffer {}
unsafe impl<T: Copy + Sync> Sync for Buffer<T> {}
unsafe impl<T: Copy + Send> Send for Buffer<T> {}

impl Default for Buffer {
impl<T: Copy> Default for Buffer<T> {
fn default() -> Self {
Self::from(vec![])
}
}

impl Deref for Buffer {
type Target = [u8];
fn deref(&self) -> &[u8] {
unsafe { slice::from_raw_parts(self.data as *const u8, self.len) }
impl<T: Copy> Deref for Buffer<T> {
type Target = [T];
fn deref(&self) -> &[T] {
unsafe { slice::from_raw_parts(self.data as *const T, self.len) }
}
}

impl DerefMut for Buffer {
fn deref_mut(&mut self) -> &mut [u8] {
impl<T: Copy> DerefMut for Buffer<T> {
fn deref_mut(&mut self) -> &mut [T] {
unsafe { slice::from_raw_parts_mut(self.data, self.len) }
}
}

impl Buffer {
impl<T: Copy> Buffer<T> {
pub(super) fn new() -> Self {
Self::default()
}
Expand All @@ -53,7 +55,7 @@ impl Buffer {
// because in the case of small arrays, codegen can be more efficient
// (avoiding a memmove call). With extend_from_slice, LLVM at least
// currently is not able to make that optimization.
pub(super) fn extend_from_array<const N: usize>(&mut self, xs: &[u8; N]) {
pub(super) fn extend_from_array<const N: usize>(&mut self, xs: &[T; N]) {
if xs.len() > (self.capacity - self.len) {
let b = self.take();
*self = (b.reserve)(b, xs.len());
Expand All @@ -64,7 +66,7 @@ impl Buffer {
}
}

pub(super) fn extend_from_slice(&mut self, xs: &[u8]) {
pub(super) fn extend_from_slice(&mut self, xs: &[T]) {
if xs.len() > (self.capacity - self.len) {
let b = self.take();
*self = (b.reserve)(b, xs.len());
Expand All @@ -75,7 +77,7 @@ impl Buffer {
}
}

pub(super) fn push(&mut self, v: u8) {
pub(super) fn push(&mut self, v: T) {
// The code here is taken from Vec::push, and we know that reserve()
// will panic if we're exceeding isize::MAX bytes and so there's no need
// to check for overflow.
Expand All @@ -90,7 +92,7 @@ impl Buffer {
}
}

impl Write for Buffer {
impl Write for Buffer<u8> {
fn write(&mut self, xs: &[u8]) -> io::Result<usize> {
self.extend_from_slice(xs);
Ok(xs.len())
Expand All @@ -106,35 +108,35 @@ impl Write for Buffer {
}
}

impl Drop for Buffer {
impl<T: Copy> Drop for Buffer<T> {
fn drop(&mut self) {
let b = self.take();
(b.drop)(b);
}
}

impl From<Vec<u8>> for Buffer {
fn from(mut v: Vec<u8>) -> Self {
impl<T: Copy> From<Vec<T>> for Buffer<T> {
fn from(mut v: Vec<T>) -> Self {
let (data, len, capacity) = (v.as_mut_ptr(), v.len(), v.capacity());
mem::forget(v);

// This utility function is nested in here because it can *only*
// be safely called on `Buffer`s created by *this* `proc_macro`.
fn to_vec(b: Buffer) -> Vec<u8> {
fn to_vec<T: Copy>(b: Buffer<T>) -> Vec<T> {
unsafe {
let Buffer { data, len, capacity, .. } = b;
mem::forget(b);
Vec::from_raw_parts(data, len, capacity)
}
}

extern "C" fn reserve(b: Buffer, additional: usize) -> Buffer {
extern "C" fn reserve<T: Copy>(b: Buffer<T>, additional: usize) -> Buffer<T> {
let mut v = to_vec(b);
v.reserve(additional);
Buffer::from(v)
}

extern "C" fn drop(b: Buffer) {
extern "C" fn drop<T: Copy>(b: Buffer<T>) {
mem::drop(to_vec(b));
}

Expand Down
4 changes: 2 additions & 2 deletions library/proc_macro/src/bridge/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ pub struct Client<I, O> {
// a wrapper `fn` pointer, once `const fn` can reference `static`s.
pub(super) get_handle_counters: extern "C" fn() -> &'static HandleCounters,

pub(super) run: extern "C" fn(Bridge<'_>) -> Buffer,
pub(super) run: extern "C" fn(Bridge<'_>) -> Buffer<u8>,

pub(super) _marker: PhantomData<fn(I) -> O>,
}
Expand All @@ -392,7 +392,7 @@ impl<I, O> Clone for Client<I, O> {
fn run_client<A: for<'a, 's> DecodeMut<'a, 's, ()>, R: Encode<()>>(
mut bridge: Bridge<'_>,
f: impl FnOnce(A) -> R,
) -> Buffer {
) -> Buffer<u8> {
// The initial `cached_buffer` contains the input.
let mut buf = bridge.cached_buffer.take();

Expand Down
4 changes: 2 additions & 2 deletions library/proc_macro/src/bridge/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,10 +226,10 @@ use rpc::{Decode, DecodeMut, Encode, Reader, Writer};
pub struct Bridge<'a> {
/// Reusable buffer (only `clear`-ed, never shrunk), primarily
/// used for making requests, but also for passing input to client.
cached_buffer: Buffer,
cached_buffer: Buffer<u8>,

/// Server-side function that the client uses to make requests.
dispatch: closure::Closure<'a, Buffer, Buffer>,
dispatch: closure::Closure<'a, Buffer<u8>, Buffer<u8>>,

/// If 'true', always invoke the default panic hook
force_show_panics: bool,
Expand Down
2 changes: 1 addition & 1 deletion library/proc_macro/src/bridge/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::num::NonZeroU32;
use std::ops::Bound;
use std::str;

pub(super) type Writer = super::buffer::Buffer;
pub(super) type Writer = super::buffer::Buffer<u8>;

pub(super) trait Encode<S>: Sized {
fn encode(self, w: &mut Writer, s: &mut S);
Expand Down
30 changes: 15 additions & 15 deletions library/proc_macro/src/bridge/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,12 @@ macro_rules! define_dispatcher_impl {
pub trait DispatcherTrait {
// HACK(eddyb) these are here to allow `Self::$name` to work below.
$(type $name;)*
fn dispatch(&mut self, buf: Buffer) -> Buffer;
fn dispatch(&mut self, buf: Buffer<u8>) -> Buffer<u8>;
}

impl<S: Server> DispatcherTrait for Dispatcher<MarkedTypes<S>> {
$(type $name = <MarkedTypes<S> as Types>::$name;)*
fn dispatch(&mut self, mut buf: Buffer) -> Buffer {
fn dispatch(&mut self, mut buf: Buffer<u8>) -> Buffer<u8> {
let Dispatcher { handle_store, server } = self;

let mut reader = &buf[..];
Expand Down Expand Up @@ -121,10 +121,10 @@ pub trait ExecutionStrategy {
fn run_bridge_and_client(
&self,
dispatcher: &mut impl DispatcherTrait,
input: Buffer,
run_client: extern "C" fn(Bridge<'_>) -> Buffer,
input: Buffer<u8>,
run_client: extern "C" fn(Bridge<'_>) -> Buffer<u8>,
force_show_panics: bool,
) -> Buffer;
) -> Buffer<u8>;
}

pub struct SameThread;
Expand All @@ -133,10 +133,10 @@ impl ExecutionStrategy for SameThread {
fn run_bridge_and_client(
&self,
dispatcher: &mut impl DispatcherTrait,
input: Buffer,
run_client: extern "C" fn(Bridge<'_>) -> Buffer,
input: Buffer<u8>,
run_client: extern "C" fn(Bridge<'_>) -> Buffer<u8>,
force_show_panics: bool,
) -> Buffer {
) -> Buffer<u8> {
let mut dispatch = |buf| dispatcher.dispatch(buf);

run_client(Bridge {
Expand All @@ -157,10 +157,10 @@ impl ExecutionStrategy for CrossThread1 {
fn run_bridge_and_client(
&self,
dispatcher: &mut impl DispatcherTrait,
input: Buffer,
run_client: extern "C" fn(Bridge<'_>) -> Buffer,
input: Buffer<u8>,
run_client: extern "C" fn(Bridge<'_>) -> Buffer<u8>,
force_show_panics: bool,
) -> Buffer {
) -> Buffer<u8> {
use std::sync::mpsc::channel;

let (req_tx, req_rx) = channel();
Expand Down Expand Up @@ -194,10 +194,10 @@ impl ExecutionStrategy for CrossThread2 {
fn run_bridge_and_client(
&self,
dispatcher: &mut impl DispatcherTrait,
input: Buffer,
run_client: extern "C" fn(Bridge<'_>) -> Buffer,
input: Buffer<u8>,
run_client: extern "C" fn(Bridge<'_>) -> Buffer<u8>,
force_show_panics: bool,
) -> Buffer {
) -> Buffer<u8> {
use std::sync::{Arc, Mutex};

enum State<T> {
Expand Down Expand Up @@ -260,7 +260,7 @@ fn run_server<
handle_counters: &'static client::HandleCounters,
server: S,
input: I,
run_client: extern "C" fn(Bridge<'_>) -> Buffer,
run_client: extern "C" fn(Bridge<'_>) -> Buffer<u8>,
force_show_panics: bool,
) -> Result<O, PanicMessage> {
let mut dispatcher =
Expand Down