Make constraints explicitly constant and evaluated in compile time & move some computation there (OER/PER)

[Nicceboy]

Small performance improvement again.

Seems like the compiler does not currently evaluate constant constraints in compile-time.

It helps if we move the constraint construction to separate expression like follows

#[allow(clippy::mutable_key_type)]
impl rasn::Encode for Test {
    fn encode_with_tag_and_constraints<'constraints, EN: rasn::Encoder>(
        &self,
        encoder: &mut EN,
        tag: rasn::Tag,
        constraints: rasn::types::Constraints<'constraints>,
    ) -> core::result::Result<(), EN::Error> {
        #[allow(unused)]
        let __rasn_field_r = &self.r;
        #[allow(unused)]
        let __rasn_field_g = &self.g;
        #[allow(unused)]
        let __rasn_field_b = &self.b;
        #[allow(unused)]
        let __rasn_field_a = &self.a;
        encoder
            .encode_sequence::<Self, _>(tag, |encoder| {
                const CONSTRAINT_0: rasn::types::Constraints =
                    rasn::types::Constraints::new(&[rasn::types::Constraint::Value(
                        rasn::types::constraints::Extensible::new(
                            rasn::types::constraints::Value::new(
                                rasn::types::constraints::Bounded::const_new(
                                    0i128 as i128,
                                    255i128 as i128,
                                ),
                            ),
                        )
                        .set_extensible(false),
                    )]);
                self.r.encode_with_tag_and_constraints(
                    encoder,
                    rasn::Tag::new(rasn::types::Class::Context, 0usize as u32),
                    <u8 as rasn::AsnType>::CONSTRAINTS.override_constraints(CONSTRAINT_0),
                )?;
                const CONSTRAINT_1: rasn::types::Constraints =
                    rasn::types::Constraints::new(&[rasn::types::Constraint::Value(
                        rasn::types::constraints::Extensible::new(
                            rasn::types::constraints::Value::new(
                                rasn::types::constraints::Bounded::const_new(
                                    0i128 as i128,
                                    255i128 as i128,
                                ),
                            ),
                        )
                        .set_extensible(false),
                    )]);
                self.g.encode_with_tag_and_constraints(
                    encoder,
                    rasn::Tag::new(rasn::types::Class::Context, 1usize as u32),
                    <u8 as rasn::AsnType>::CONSTRAINTS.override_constraints(CONSTRAINT_1),
                )?;
                const CONSTRAINT_2: rasn::types::Constraints =
                    rasn::types::Constraints::new(&[rasn::types::Constraint::Value(
                        rasn::types::constraints::Extensible::new(
                            rasn::types::constraints::Value::new(
                                rasn::types::constraints::Bounded::const_new(
                                    0i128 as i128,
                                    255i128 as i128,
                                ),
                            ),
                        )
                        .set_extensible(false),
                    )]);
                self.b.encode_with_tag_and_constraints(
                    encoder,
                    rasn::Tag::new(rasn::types::Class::Context, 2usize as u32),
                    <u8 as rasn::AsnType>::CONSTRAINTS.override_constraints(CONSTRAINT_2),
                )?;
                const CONSTRAINT_3: rasn::types::Constraints =
                    rasn::types::Constraints::new(&[rasn::types::Constraint::Value(
                        rasn::types::constraints::Extensible::new(
                            rasn::types::constraints::Value::new(
                                rasn::types::constraints::Bounded::const_new(
                                    0i128 as i128,
                                    65335i128 as i128,
                                ),
                            ),
                        )
                        .set_extensible(false),
                    )]);
                self.a.encode_with_tag_and_constraints(
                    encoder,
                    rasn::Tag::new(rasn::types::Class::Context, 3usize as u32),
                    <u16 as rasn::AsnType>::CONSTRAINTS.override_constraints(CONSTRAINT_3),
                )?;
                Ok(())
            })
            .map(drop)
    }
}

OER/BER gets performance boost immediately, but PER decoding gets regression. Need to investigate that still.

[Nicceboy]

Seems like that the previous constant evaluation problem applies only for default release builds, lto will evaluate constants correctly. Maybe that part of the change is not necessary.

[XAMPPRocky]

I think it's fine to have this optimisation, even if it's only for default release, because that is what most people use, since it's the default.

[XAMPPRocky]

I wonder if there's a benefit to making override_constraints const and removing that evaluation from runtime.

[Nicceboy]

It is quite hot function. I tried to make it const but there were some challenges related to Cow type. Another hot function is

rasn/src/types/constraints.rs

Lines 45 to 47 in 4e10e6b

	pub fn value(&self) -> Option<Extensible<Value>> {
	self.0.iter().find_map(|constraint| constraint.to_value())
	}

which in theory could be also constant/pre-computed.

[Nicceboy]

Would it be okay if I try change the constraints type to be

#[derive(Debug, Clone)]
pub struct Constraints(pub &'static [Constraint])

I think there isn't any use case where it should not be possible to calculate the constraint in compile time?

[XAMPPRocky]

I don't think that will work with having types that wrap a constrained type and add another constraint.

[Nicceboy]

I managed to make Constraints almost completely constant but then I noticed that this cannot support use cases like the following, which is not currently included in tests

#[derive(AsnType, Debug, Decode, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
#[rasn(automatic_tags)]
pub struct ContributedExtensionBlock<T>
where
    T: ContributedExtension,
{
    id: HeaderInfoContributorId,
    #[rasn(size("1.."))]
    extn: SequenceOf<T>,
}

Constraints are derived from the type SequenceOf<T> and we can't create constants from that.

Should we try to identify if there is generic present when creating the constant, or just drop the support for now? It would have given quite big performance boost. It would have been the next bottleneck after reducing allocations. I haven't checked yet how much PER calculations could have been moved there.

I also noticed that I have posted screenshots with different compiler versions, so the following is with the current stable on M2:

[XAMPPRocky]

Constraints are derived from the type SequenceOf and we can't create constants from that.

Could you explain why? I'm not sure I understand why you can't.

[Nicceboy]

Constraints are derived from the type SequenceOf and we can't create constants from that.

Could you explain why? I'm not sure I understand why you can't.

Here is the relevant compiler error:

181 | pub struct ContributedExtensionBlock<T>
    |                                      - type parameter from outer item
...
187 |     extn: SequenceOf<T>,
    |                      ^ use of generic parameter from outer item
    |
    = note: a `const` is a separate item from the item that contains it

Generated code would look like:

#[allow(clippy::mutable_key_type)]
impl<T: rasn::Encode> rasn::Encode for ContributedExtensionBlock<T>
where
    T: ContributedExtension,
{
    fn encode_with_tag_and_constraints<EN: rasn::Encoder>(
        &self,
        encoder: &mut EN,
        tag: rasn::Tag,
        constraints: rasn::types::Constraints,
    ) -> core::result::Result<(), EN::Error> {
        #[allow(unused)]
        let __rasn_field_id = &self.id;
        #[allow(unused)]
        let __rasn_field_extn = &self.extn;
        encoder
            .encode_sequence::<Self, _>(tag, |encoder| {
                self.id.encode_with_tag(
                    encoder,
                    rasn::Tag::new(rasn::types::Class::Context, 0usize as u32),
                )?;
                const FIELD_CONSTRAINT_1: rasn::types::Constraints =
                    rasn::types::Constraints::from_fixed_size(
                        &<SequenceOf<T> as rasn::AsnType>::CONSTRAINTS.merge(
                            rasn::types::Constraints::new(&[rasn::types::Constraint::Size(
                                rasn::types::constraints::Extensible::new(
                                    rasn::types::constraints::Size::new(
                                        rasn::types::constraints::Bounded::start_from(
                                            1i128 as usize,
                                        ),
                                    ),
                                )
                                .set_extensible(false),
                            )]),
                        ),
                    );
                self.extn.encode_with_tag_and_constraints(
                    encoder,
                    rasn::Tag::new(rasn::types::Class::Context, 1usize as u32),
                    FIELD_CONSTRAINT_1,
                )?;
                Ok(())
            })
            .map(drop)
    }
}

The issue is on part <SequenceOf<T> as rasn::AsnType>::CONSTRAINTS, where the constant is coming from the outer item. Or is there some way to get around this?

[Nicceboy]

The above use-case is mainly related to use of information object classes and information object sets - however I checked the compiler crate and it does not use generics but rather just Any type. Do we need to support generics? The above limitation can be also bypassed by writing own encode/decode function to not mark type as constant, if generics are needed.

[XAMPPRocky]

Do we need to support generics?

I'm hesitant to take away that functionality.

I wonder if we could solve this by approaching it from a different way.

What if the field constraints constants were specified in the Constructed trait? That way they could be created and resolved and it shouldn't cause an issue.

I made a small prototype to test if it could resolve the constants, and it seems like it works.

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=d1821ee0cb3f19ef72eb0fe027b2fe7f

[Nicceboy]

I don't think that completely explicit constant constraints are possible after all.

ASN.1 standard allows newtyping the type up to infinity and each new type can introduce additional constraints. There is a bunch of set theory how to calculate the final constraints.
The problem isn't simple one: https://ieeexplore.ieee.org/document/8129780
Constraints should be always passed recursively and parameters from the caller should be noted.

This isn't currently properly noted in all scenarios and I also did not note it so I was able to make it constant (excluding the generics). I think I will just improve the situation slightly on this PR and I cannot solve it completely for now.

[Nicceboy]

I believe this can be reviewed.

We might need to add tracking issue that rasn currently does not support all the possible cases of constraints.

For example, the effective size constraint can consist from multiple ranges and single value constraints, so that overall constraint value is not continuous.
I did not do this change, since it appeared to be rather challenging and needs much more changes. It is rarer use-case anyway.

The current improvement is that constraints are almost completely constant, and it also makes proper intersection instead of just overriding. OER relies more directly on constraints, and gets significant performance improvements. Difference to PER starts to be rather large and the changes are not so visible.

The constraint code is also simplified a bit - we don't need that much generics since types are limited by the implemented types in Bounded type.

Making constraints constants with the generics associated types appeared to be impossible with the current stable Rust. This has something do with the order of constant evaluation and generic monomorphization, and therefore the constant value cannot be known at the correct time.

The problem was a bit different from the playground sample - the issue is that constraints must be calculated outside of trait definition with generic parameters, so we don't know exactly the type at time what generic will have.

So instead I added some checks that if generics are present, the constraints are less constant.
The code is also quite bit different and now the constant values do not matter that much.

Performance boost with integer test (no extensions):

[XAMPPRocky]

Thank you for your PR!

[XAMPPRocky]

We might need to add tracking issue that rasn currently does not support all the possible cases of constraints.

Would you mind making the issue for this?