WIP: Add AppWorkload builder & Cycle checking #4
Conversation
Co-authored-by: leudz <dylan.ancel@gmail.com>
| pub fn add_plugin_workload<P>(&mut self, plugin: P) -> AppWorkload | ||
| where | ||
| P: Plugin + 'static, |
There was a problem hiding this comment.
I usually put the generics right in the definition if the line doesn't get too wide:
| pub fn add_plugin_workload<P>(&mut self, plugin: P) -> AppWorkload | |
| where | |
| P: Plugin + 'static, | |
| pub fn add_plugin_workload<P: Plugin + 'static>(&mut self, plugin: P) -> AppWorkload |
There was a problem hiding this comment.
The following fn (add_plugin_workload_with_info) has the same structure but does become too wide for one line, so I'm leaning towards the two signatures reading the same way.
| self.add_plugin_workload_with_info(plugin).0 | ||
| } | ||
|
|
||
| #[track_caller] |
There was a problem hiding this comment.
Does trace_span! obey #[track_caller]? Or is there any possible panic I overlooked?
There was a problem hiding this comment.
builder.finish_with_info_named unwraps on the "add to world" step
| use crate::{App, AppWorkload, AppWorkloadInfo, PluginAssociated, TypeIdBuckets}; | ||
|
|
||
| #[derive(Clone)] | ||
| pub struct CyclePluginAssociations { |
There was a problem hiding this comment.
Some documentation would be nice. What's a cycle here?
There was a problem hiding this comment.
I'll add some docs!
| } | ||
| /// If you override this to return true, then you will be allowed to add your plugin multiple times. | ||
| fn can_add_multiple_times(&self) -> bool { | ||
| false | ||
| } | ||
| } |
There was a problem hiding this comment.
If we decide to create a macro to build an impl for this, here would be a good place to put it.
There was a problem hiding this comment.
Hmmm... I feel like there's some niceties we could apply using a DSL of some kind, but the impls are pretty lightweight overall IMO.
| } | ||
| } | ||
|
|
||
| unsafe impl<'a, T: 'static + Send + Sync> BorrowInfo for Tracked<'a, T> { |
There was a problem hiding this comment.
This is an interesting concept; but I don't understand what BorrowInfo does, yet. Note to self: Need to look into this at some point.
There was a problem hiding this comment.
Don't look too closely right now since it's being reworked a bit by @leudz.
There was a problem hiding this comment.
BorrowInfo collects information about what each system borrows, this is used by WorkloadBuilder to decide scheduling inside a workload.
The borrow_info function was previously in Borrow, I moved it to its own trait to make it unsafe. I will add a way to bypass locking and trust the scheduler.
But the scheduler can only do correctly its job if BorrowInfo is implemented correctly.
| enum InnerTrackedState { | ||
| New, | ||
| Modified, | ||
| NoChanges, |
There was a problem hiding this comment.
| NoChanges, | |
| Unchanged, |
There was a problem hiding this comment.
Any precedent for this rename or just preference?
|
|
||
| #[inline(always)] | ||
| fn deref(&self) -> &T { | ||
| &self.0 .1 |
There was a problem hiding this comment.
load-bearing whitespace? Is some parser confused and thinks this was a float? rustc would accept &self.0.1.
There was a problem hiding this comment.
cargo fmt / rustfmt did this, I think.
|
Oops! I just saw that you already merged. So perhaps there should be a followup PR. |
Changes to enable detection of workload conflicts.