RAII type for console.time and console.timeEnd about gloo HOT 16 CLOSED

I would be in favor of extending @cbrevik's ConsoleTimer API with the following

pub fn time_scope<F, T>(label: &str, f: F) -> T
where
    F: FnOnce() -> T;

but I think we must have the RAII struct.

Happy to bikeshed on naming as well.

from gloo.

I think that having the core struct be the RAII type is just fine. For those who want to use it that way, it is easily accessible. We should do as @fitzgen mentioned and expose the closure-based form as well, though, as something like ConsoleTimer::scope("xyz", || …). In documentation, we should prefer to demonstrate using the closure form. For cases where this doesn't work, or the timer needs to be passed into another scope (such as a delayed action) the struct form is still necessary.

from gloo.

Summary

Add RAII type for console.time and console.timeEnd.

Motivation

Measuring invocation time of code blocks can be done with console.time and console.timeEnd, available via the web-sys crate. This requires two calls, a console.timeEnd call for every console.time call. Wrapping this up in a RAII type would make the measurement code less verbose. It would also make it make it more safe, since you ensure that console.timeEnd is called when the object goes out of scope.

Detailed Explanation

As noted, the implementation could be quite similar to the type from the Rust/Wasm book:

extern crate web_sys;
use web_sys::console;

pub struct ConsoleTimer<'a> {
    label: &'a str,
}

impl<'a> ConsoleTimer<'a> {
    pub fn new(label: &'a str) -> ConsoleTimer<'a> {
        console::time_with_label(label);
        ConsoleTimer { label }
    }
}

impl<'a> Drop for ConsoleTimer<'a> {
    fn drop(&mut self) {
        console::time_end_with_label(self.label);
    }
}

Then this type can be constructed at the top of a code block in order to do a measurement:

{
    let _timer = ConsoleTimer::new("my_unique_label"); // measurement starts
    // the code under measurement
} // measurement ends

Drawbacks, Rationale, and Alternatives

Although a simple way to perform measurements without too much orchestration, this type might be confusing to some developers? It might not immediately be apparant that you don't have to call some start or end-method for measurements.

Unresolved Questions

• Unsure about naming here? ConsoleTimer might be to generic a name for this use case? Maybe CodeBlockTimer is a more descriptive name?
• Should we allow a timer without a label? I.e. console::time() and console::time_end.
• There is no check if a timer with the label already exists, but unsure if such a check is even possible at this time?

from gloo.

Additionally, it is easy to implement the explicit scope version with the RAII building block:

fn with_console_time<F, T>(label: &str, scope: F) -> T
where
    F: FnOnce() -> T
{
    let _timer = ConsoleTimer::new(label);
    f()
}

// usage...
with_console_timer("foobar", || {
    // ...
});

But you can't implement RAII in terms of callbacks or scope macros.

from gloo.

An RAII type can also be used for asynchronous tasks, while a closure can't be (maybe eventually with async/await syntax, but not right now).

from gloo.

We haven't figured out the answer to all these questions yet (e.g. see #27) but in this case I think having it in its own gloo-console-timer crate makes sense.

I think we are ready for a PR here! :)

from gloo.

Given that we already have an example implementation from the book, I think this would be a good issue for someone to make a quick strawman API proposal like #10 (comment), and then get a PR going!

from gloo.

I'd like to take a stab at this!

from gloo.

In my own personal projects, I've used a macro for this:

console_time!("Foo", {
    // code goes here
})

I personally think this is more ergonomic and intuitive, compared to an RAII type that has side effects on creation and dropping.

from gloo.

As a relative novice at Rust, that pattern immediately seems more explicit and intuitive to me as well @Pauan

Edit: Something like this?

from gloo.

@cbrevik I would just define it like this, without bothering with ConsoleTimer at all:

#[macro_export]
macro_rules! console_time {
    ($label:expr, $b:block) => {
        let label = $label;
        console::time_with_label(label);
        let output = $b;
        console::time_end_with_label(label);
        output
    };
}

But it doesn't matter too much, one way or the other.

from gloo.

I personally think this is more ergonomic and intuitive, compared to an RAII type that has side effects on creation and dropping.

Interesting. RAII is a pattern that is commonly used in Rust, and seems very familiar to me: std::fs::File, std::sync::Mutex, std::rc::Rc, tempfile::TempDir, etc.

Even when explicit control blocks are used, it is usually done via a closure rather than a macro, e.g. crossbeam_utils::thread::scope(|s| { ... }) instead of crossbeam_utils::thread::scope!(s, { .. }).

from gloo.

Interesting. RAII is a pattern that is commonly used in Rust, and seems very familiar to me: std::fs::File, std::sync::Mutex, std::rc::Rc, tempfile::TempDir, etc.

The difference is that in all of those cases you actually use the variable.

In the case of ConsoleTimer you always do let _timer = ConsoleTimer::new("foo"); and then never touch it again.

RAII is great, but it's not always the right tool for the job. Using a macro or closure based API is better in this case.

However, I agree with you that there are some edge cases where RAII is useful (putting into a struct to log how long an object lives, using it with Futures 0.1 to benchmark async code, etc.)

So I agree that we should have an RAII type, but it should be put into a raw submodule.

from gloo.

I like the idea of having the scoped callback version be a static method on the RAII type 👍

from gloo.

I like it! That makes sense to me as well.

I have a semi-related dumb question; would it make sense to have this suggested API in its own specific crate, e.g. gloo-console-timer? (Awaiting bikeshedding on naming.) Or would it make more sense to put it in a gloo-console crate where the API can be expanded to include more console related functionality?

This sentence from the README sort of makes me think it should be in its own crate:

Carefully crafting a tiny Wasm module and integrating it back into an existing JavaScript project? Grab that one targeted library you need out from the toolkit and use it by itself.

But I'm not clear on how granular it should be.

from gloo.

Cool, will get on it! :)

from gloo.