Add is_compatible method to event content enums to fix Any*Event deserialization

[DevinR528]

Will something like this work?

[jplatte]

I wouldn't want to hardcode more knowledge about the specific event types in the macros. I was thinking more of adding methods to the generated event content enums to determine which variant to deserialize.

[DevinR528]

This would still require a manual/custom derive for the Any*Event enums to be deserialized so the new method could be called?

[jplatte]

I'm not sure it's worth making it a macro. It could just be one implementation for AnyEvent, one for AnyRoomEvent.

[DevinR528]

How do I get at the actual deserialized content?

// inside of AnyEvent Deserialize impl
let json = JsonValue::deserialize(deserializer)?;
let raw: Box<RawJsonValue> = util::get_field(&json, "content")?;
let ev_type: String = util::get_field(&json, "type")?;
let content: ?????? =
    ruma_events::EventContent::from_parts(&ev_type, raw).map_err(D::Error::custom)?;
// then call the Any*EventContent's new method to figure out the variant

Do I need to manually do what #[serde(untagged)] did for all the Any*EventContent enums as in if let Ok(content) = ...? Or is there a better way to go about this?

[jplatte]

I was thinking of something like

// for AnyRoomEvent
match ev_type.as_str() {
    "m.room.redaction" => Self::Redaction(from_json_value(json).map_err(...)?),
    t if AnyMessageEventContent::is_compatible(t) => Self::Message(from_json_value(json).map_err(...)?),
    t if AnyStateEventContent::is_compatible(t) => Self::State(from_json_value(json).map_err(...)?),
}

[jplatte]

❓

[DevinR528]

criterion generates a ruma-events/target dir I figured this should be ignored?

[jplatte]

Sure. I don't like it doing that, but I guess we can't do much about it.

[jplatte]

I don't understand this comment. What's "the default cargo bench"? Also, it's not clear to me what is avoided.

[jplatte]

I'm guessing this is about the -- --save-baseline syntax, but that's just an established convention for unix-style CLI tools – don't process stuff after a -- as flags / options. We don't need to document that here.

[DevinR528]

cargo bench works but if you use criterions suggestion cargo bench -- --save-baseline <name> it fails with the error cargo bench unknown option --save-baseline I'm assuming this has something to do with the fact that we are in a workspace? I should change it to say the above and that the workaround is
cargo bench --bench event_deserialize -- --save-baseline <name>

[jplatte]

I don't know what that error is about. Does it make a difference whether you run that command from the repo root vs ruma-events?

[DevinR528]

Nope at first, I thought it did but it gave me the same error both times. I've had this happen before turning off the bench harness worked the other time I found this and used the [lib] harness = false so I have no idea exactly why it won't work but I'd figure workspaces still being a bit flaky with some things.

[DevinR528]

This is what I have now

// `cargo bench` works but if you use `cargo bench -- --save-baseline <name>`
// or pass any other args to it it fails with the error
// `cargo bench unknown option --save-baseline`.
// To pass args to criterion use this form
// `cargo bench --bench <name of the bench> -- --save-baseline <name>`

[jplatte]

Looks pretty good. What do the benchmark results look like on your machine?

[DevinR528]

deserialize to `AnyEvent`                                                                             
                        time:   [12.192 us 12.216 us 12.244 us]
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) high mild

deserialize to `AnyRoomEvent`                                                                             
                        time:   [11.973 us 11.986 us 12.000 us]
Found 2 outliers among 100 measurements (2.00%)
  1 (1.00%) high mild
  1 (1.00%) high severe

deserialize to `AnyStateEvent`                                                                             
                        time:   [6.9297 us 6.9388 us 6.9498 us]
Found 3 outliers among 100 measurements (3.00%)
  3 (3.00%) high mild

deserialize to `StateEvent<PowerLevelsEventContent>`                                                                             
                        time:   [6.8239 us 6.8303 us 6.8365 us]
Found 1 outliers among 100 measurements (1.00%)
  1 (1.00%) low mild

So fully generic is about 1/2 as fast as even SpecificEvent<AnyContent>.

The MessageEvent<MessageEventContent> bench that was removed was 4.something us