Skip to content

Reduce message cloning operations #190

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

Open
zainkabani wants to merge 41 commits into
base: main
Choose a base branch
from
Open

Reduce message cloning operations #190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
d7bf2de
initial commit
zainkabani Oct 14, 2022
b42c33c
fix typo
zainkabani Oct 14, 2022
3ae2953
use cursor for parse params instead of bytesmut
zainkabani Oct 14, 2022
33a0cad
undo
zainkabani Oct 14, 2022
65bd10d
Update to use a dedicated server message buffer
zainkabani Oct 17, 2022
09db31d
fmt
zainkabani Oct 17, 2022
8b47f32
Read message directly onto buffer instead of new bytesmut
zainkabani Nov 15, 2022
d6c8271
Merge branch 'main' into zain/reduce-cloning-operations
zainkabani Nov 15, 2022
1589548
remove commented code
zainkabani Nov 15, 2022
e2b2cb0
Move server buffer to server object
zainkabani Nov 16, 2022
8d68d22
Move client and server message buffers to attribute of object
zainkabani Nov 16, 2022
f019e28
Remove clear buffer function since public already
zainkabani Nov 16, 2022
4cb38b8
Remove commented code
zainkabani Nov 16, 2022
8f3fc25
Remove cloning operation for query router
zainkabani Nov 16, 2022
728f6cf
Rename server and client buffers to buffer
zainkabani Nov 16, 2022
ee19dbc
Fix bug
zainkabani Nov 16, 2022
027011d
Rename read_messages to read_messages_into_buffer
zainkabani Nov 16, 2022
f0ceb8b
fmt
zainkabani Nov 16, 2022
021a108
Rename to message_buffer to be more explicit
zainkabani Nov 16, 2022
24d36af
Read message body directly onto buffer
zainkabani Nov 16, 2022
8a58f8b
Fix bug
zainkabani Nov 16, 2022
089348b
fmt
zainkabani Nov 16, 2022
ecb9627
Unused try_into
zainkabani Nov 16, 2022
a3e3801
use read_exact instead of read to guarantee number of bytes read
zainkabani Nov 16, 2022
bece3b9
Remove length check
zainkabani Nov 17, 2022
54ed58e
fmt
zainkabani Nov 17, 2022
ada475e
Merge branch 'main' into zain/reduce-cloning-operations
zainkabani Nov 17, 2022
ccde8f0
Add comment to read_string function and uses sizeof for byte lengths …
zainkabani Nov 21, 2022
6c5b5df
fmt
zainkabani Nov 21, 2022
7bf9503
Refactor reading query in infer role
zainkabani Nov 21, 2022
5f8473c
Creates send server message to client function to both send to client…
zainkabani Nov 22, 2022
0fa0214
Merge branch 'main' into zain/reduce-cloning-operations
zainkabani Nov 28, 2022
2986f06
Merge branch 'main' into zain/reduce-cloning-operations
zainkabani Nov 28, 2022
382e0b8
Ensures buffer is cleared before it checked in
zainkabani Nov 28, 2022
459e5d4
revert?
zainkabani Nov 28, 2022
ae4078e
try again
zainkabani Nov 29, 2022
08f179c
Merge branch 'main' into zain/reduce-cloning-operations
zainkabani Dec 21, 2022
6ad7564
Merge branch 'main' into zain/reduce-cloning-operations
zainkabani Dec 21, 2022
2a84549
Update comments
zainkabani Dec 21, 2022
fb891fc
fmt
zainkabani Dec 21, 2022
9937aba
fix
zainkabani Dec 21, 2022
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
21 changes: 10 additions & 11 deletions src/admin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@ use tokio::time::Instant;
use crate::config::{get_config, reload_config, VERSION};
use crate::errors::Error;
use crate::messages::*;
use crate::pool::get_all_pools;
use crate::pool::{get_all_pools, ClientServerMap};
use crate::stats::{
get_address_stats, get_client_stats, get_pool_stats, get_server_stats, ClientState, ServerState,
};
use crate::ClientServerMap;

pub fn generate_server_info_for_admin() -> BytesMut {
let mut server_info = BytesMut::new();
Expand Down Expand Up @@ -171,7 +170,7 @@ where
res.put_i32(5);
res.put_u8(b'I');

write_all_half(stream, res).await
write_all_half(stream, &res).await
Copy link
Copy Markdown
Contributor

@levkk levkk Oct 14, 2022

Choose a reason for hiding this comment

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

I don't think this changes anything. res is deallocated at the end of this block anyway, so borrowing it doesn't do anything, you might as well pass ownership to write_all_half(). Passing ownership != copying.

Copy link
Copy Markdown
Contributor Author

@zainkabani zainkabani Oct 14, 2022

Choose a reason for hiding this comment

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

if the function doesn't need the explicit object then it's better to have it accept the reference so it can stay more flexible

Copy link
Copy Markdown
Contributor Author

@zainkabani zainkabani Nov 16, 2022

Choose a reason for hiding this comment

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

Another reason we're using reference here is because we don't want to clone or pass ownership of the buffer to the send operation so we pass a reference instead.

}

/// Show PgCat version.
Expand All @@ -189,7 +188,7 @@ where
res.put_i32(5);
res.put_u8(b'I');

write_all_half(stream, res).await
write_all_half(stream, &res).await
}

/// Show utilization of connection pools for each shard and replicas.
Expand Down Expand Up @@ -250,7 +249,7 @@ where
res.put_i32(5);
res.put_u8(b'I');

write_all_half(stream, res).await
write_all_half(stream, &res).await
}

/// Show shards and replicas.
Expand Down Expand Up @@ -317,7 +316,7 @@ where
res.put_i32(5);
res.put_u8(b'I');

write_all_half(stream, res).await
write_all_half(stream, &res).await
}

/// Ignore any SET commands the client sends.
Expand Down Expand Up @@ -349,7 +348,7 @@ where
res.put_i32(5);
res.put_u8(b'I');

write_all_half(stream, res).await
write_all_half(stream, &res).await
}

/// Shows current configuration.
Expand Down Expand Up @@ -395,7 +394,7 @@ where
res.put_i32(5);
res.put_u8(b'I');

write_all_half(stream, res).await
write_all_half(stream, &res).await
}

/// Show shard and replicas statistics.
Expand Down Expand Up @@ -455,7 +454,7 @@ where
res.put_i32(5);
res.put_u8(b'I');

write_all_half(stream, res).await
write_all_half(stream, &res).await
}

/// Show currently connected clients
Expand Down Expand Up @@ -505,7 +504,7 @@ where
res.put_i32(5);
res.put_u8(b'I');

write_all_half(stream, res).await
write_all_half(stream, &res).await
}

/// Show currently connected servers
Expand Down Expand Up @@ -559,5 +558,5 @@ where
res.put_i32(5);
res.put_u8(b'I');

write_all_half(stream, res).await
write_all_half(stream, &res).await
}
Loading