RUST-2004 Benchmark client bulk write#1293
Conversation
| .expect("invalid TARGET_ITERATION_COUNT") | ||
| }); | ||
|
|
||
| static SMALL_DOC: OnceCell<Document> = OnceCell::const_new(); |
There was a problem hiding this comment.
Nit: I think these statics can be moved into the scope of the get_ functions that use them.
| let data_path = DATA_PATH | ||
| .join("single_and_multi_document") | ||
| .join("small_doc.json"); | ||
| let mut file = spawn_blocking_and_await!(std::fs::File::open(data_path)) |
There was a problem hiding this comment.
Nit: It might be worth factoring out a utility function like
async fn from_json_file<T: DeserializeOwned>(path: impl AsRef<Path>) -> Result<T>
There was a problem hiding this comment.
makes sense, added a util function and statics for the other json files we read
| } | ||
|
|
||
| async fn do_task(&self) -> Result<()>; | ||
| async fn do_task(&mut self) -> Result<()>; |
There was a problem hiding this comment.
My understanding is that the introduction of mut here is to support copying the write models in before_task rather than do_task (which absolutely makes sense).
Personally, I'd rather avoid the mut on do_task because I think it makes it harder to reason about; how would you feel about, instead, updating the Benchmark trait with
type TaskState = ();
async fn before_task(&mut self) -> Result<TaskState> {
Ok(())
}
async fn do_task(&self, state: TaskState) -> Result<()>;
That avoids the need for mut and the Option/take ceremony, but might be more type-level complication than it's worth. WDYT?
There was a problem hiding this comment.
good idea - I never love an option/take hack like this. updated with your suggestion, it's a bit noisy to add type TaskState = () to all the types but hopefully associated type defaults will be stabilized sometime soon
| let data_path = DATA_PATH | ||
| .join("single_and_multi_document") | ||
| .join("small_doc.json"); | ||
| let mut file = spawn_blocking_and_await!(std::fs::File::open(data_path)) |
There was a problem hiding this comment.
makes sense, added a util function and statics for the other json files we read
| } | ||
|
|
||
| async fn do_task(&self) -> Result<()>; | ||
| async fn do_task(&mut self) -> Result<()>; |
There was a problem hiding this comment.
good idea - I never love an option/take hack like this. updated with your suggestion, it's a bit noisy to add type TaskState = () to all the types but hopefully associated type defaults will be stabilized sometime soon
2 participants
RUST-2004
This also includes some clippy fixes/general cleanup in the second and third commits.