Add missing docs for the rest of the code base (merge after #1026) #1028
Conversation
qinsoon
added
the
PR-testing
|
binding-refs |
|
|
src/scheduler/worker.rs
Outdated
| /// Explicitly execute a work packet in the current context (the given worker, and the current work packet). | ||
| /// The statistics collected for executing the work are counted into the current context. | ||
| /// This is NOT the normal way to execute a work packet: most work packets are polled and executed in | ||
| /// the worker's main loop ([`GCWorker::run`]). This method is only intended for cases where you would like to | ||
| /// explicitly execute a work packet under the current context. |
There was a problem hiding this comment.
Should this method be public (or exist at all)? This method is only called in one place (start_or_dispatch_scan_work), but there are more use patters of calling GCWork::do_work directly, for example, ProcessModBuf::do_work and ProcessRegionModBuf::do_work.
I think we can simply remove this method and let the user call GCWork::do_work directly if they have to, and add similar warnings to the doc comments of GCWork::do_work.
src/scheduler/worker.rs
Outdated
| /// Explicitly execute a boxed work packet in the current context. | ||
| /// See [`GCWorker::do_work`] for more information. | ||
| pub fn do_boxed_work(&'static mut self, mut work: Box<dyn GCWork<VM>>) { | ||
| work.do_work(self, self.mmtk); | ||
| } | ||
|
|
There was a problem hiding this comment.
Similar to do_work, but do_boxed_work is not used in mmtk-core at all. We may remove this function, too.
qinsoon
removed
the
PR-testing
src/scheduler/work.rs
Outdated
| /// Only call this method if you would like to explicitly execute a work packet in the current context | ||
| /// (the given worker, and the current work packet). The statistics collected for executing the work are | ||
| /// counted into the current context. This is NOT the normal way to execute a work packet: most work packets | ||
| /// are polled and executed in the worker's main loop ([`GCWorker::run`]) using `do_work_with_stat`. | ||
| /// This method should only be called directly if you would like to explicitly execute a work packet under the current context. |
There was a problem hiding this comment.
We never used the term "context" to refer to the worker and the work packet in other places, so it should be easier to understand if we don't use that word, or be more specific.
| /// Only call this method if you would like to explicitly execute a work packet in the current context | |
| /// (the given worker, and the current work packet). The statistics collected for executing the work are | |
| /// counted into the current context. This is NOT the normal way to execute a work packet: most work packets | |
| /// are polled and executed in the worker's main loop ([`GCWorker::run`]) using `do_work_with_stat`. | |
| /// This method should only be called directly if you would like to explicitly execute a work packet under the current context. | |
| /// | |
| /// Most work packets are polled and executed in the worker's main loop ([`GCWorker::run`]) | |
| /// using `do_work_with_stat`. If `do_work` is called directly during the execution of another | |
| /// work packet, bypassing `do_work_with_stat()`, this work packet will not be counted into the | |
| /// number of work packets executed, and the execution time of this work packet will be counted | |
| /// as part of the execution time of the other work packet. Only call this method directly if | |
| /// this is what you intended. But you should always consider putting the other work packet | |
| /// into a bucket so that other GC workers can execute it in parallel, unless the context- | |
| /// switching overhead is a problem. |
There was a problem hiding this comment.
I updated the comments, and used most of your suggestions.
qinsoon
added
the
PR-testing
* Update to mmtk/mmtk-core#1028 * Move `num_specialized_scans` to the binding. --------- Co-authored-by: mmtkgc-bot <mmtkgc.bot@gmail.com>
This is the last PR for adding missing docs, and it closes #309. All the public items should be documented properly now, and
#![deny(missing_docs)]is used so any new public item that does not have rustdoc will cause the build to fail.