[Perf] Reduce allocations when parsing Identifiers

[ljedrz]

The Identifier is an object that gets parsed quite often; that process involves ensuring that it is not actually a LiteralType, and in order to do so, an attempt is made to parse the identifier as that object. However, this approach is quite wasteful due to the fact that it uses alt, which clones the given input for every member of its list until a hit is found, meaning we currently perform 17 redundant allocations every time the Identifier is valid.

Since we don't actually need to do true parsing here, we can instead perform a much simpler check that doesn't need to worry about advancing the parsed input, avoiding allocations. In a local run with 4 --dev nodes, this reduces the number of allocations/s in a validator node by ~26.5%.

The extra crate that was used, enum-iterator, is small, simple, and widely used in the ecosystem. A slightly less future-proof alternative would be to introduce a const list of all the variants of the LiteralType; the basic principle is the same.

[raychu86]

I'm not too familiar with the enum-iterator APIs, but after looking at the docs, this looks correct.

[raychu86]

On a related note:

The Parse impl for LiteralType uses fixed strings for parsing. It may be preferable to use the type_name there as well:

Before:

    fn parse(string: &str) -> ParserResult<Self> {
        // Parse the type from the string.
        alt((
            map(tag("address"), |_| Self::Address),
            map(tag("boolean"), |_| Self::Boolean),
            map(tag("field"), |_| Self::Field),
            map(tag("group"), |_| Self::Group),
            ...

After:

    fn parse(string: &str) -> ParserResult<Self> {
        // Parse the type from the string.
        alt((
            map(tag(Self::Address.type_name()), |_| Self::Address),
            map(tag(Self::Boolean.type_name()), |_| Self::Boolean),
            map(tag(Self::Field.type_name()), |_| Self::Field),
            map(tag(Self::Group.type_name()), |_| Self::Group),
            ...

Not sure if there are any performance hits from this.

[ljedrz]

Not sure if there are any performance hits from this.

There shouldn't be any; I'm happy to include this one change here or separately, in a refactor PR together with similar applications - your call.

[raychu86]

It'd likely be better in a new PR 👍