Severe performance regression with combine 4

[sdroege]

I've tried porting some of my combine-based code from version 3.8.1 to 4.0.1 and it now takes very long to compile (I stopped the build after one minute). Previously it took about 10s.

This is with Rust 1.40.

Code can be found here: https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/merge_requests/260

[Marwes]

Looks like rustc bug in trait selection

Compiling the crate wasn't trivial. I had to fix some compile errors (s/Parser<Input = I/Parser<I) and dig around to find which dependencies were needed.

apt-get install libgstreamer1.0-dev libgstreamer-plugins-base1.0-dev \
      gstreamer1.0-plugins-base gstreamer1.0-plugins-good \
      gstreamer1.0-plugins-bad gstreamer1.0-plugins-ugly \
      gstreamer1.0-libav libgstrtspserver-1.0-dev

Is it possible that you could minimize the crate to something that is easier to compile? Then it ought to be possible to submit an issue to rust-lang/rust (first guess would be that changing the Input associated type to a type parameter breaks the memoization).

[sdroege]

Yeah it should be possible to move src/scc_parser.rs and src/mcc_parser.rs into a crate by themselves without any GStreamer dependencies and no other dependencies.

I'll take a look later, just wanted to put this down here first before I forget and in case it's a known problem.

[sdroege]

Also thanks for looking!

[Marwes]

A workaround may be to use the parser! or opaque! macro in a few places to break apart the huge types generated by impl Parser.

[sdroege]

Minimalized testcase:

use either::Either;

use combine;
use combine::parser::byte::hex_digit;
use combine::{choice, token};
use combine::{ParseError, Parser, RangeStream};

fn mcc_payload_item<'a, I: 'a>() -> impl Parser<I, Output = Either<u8, &'static [u8]>>
where
    I: RangeStream<Token = u8, Range = &'a [u8]>,
    I::Error: ParseError<I::Token, I::Range, I::Position>,
{
    choice!(
        token(b'G').map(|_| Either::Right([0xfau8, 0x00, 0x00].as_ref())),
        token(b'H').map(|_| Either::Right([0xfau8, 0x00, 0x00, 0xfa, 0x00, 0x00].as_ref())),
        token(b'I').map(|_| Either::Right(
            [0xfau8, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00].as_ref()
        )),
        token(b'J').map(|_| Either::Right(
            [0xfau8, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00].as_ref()
        )),
        token(b'K').map(|_| Either::Right(
            [
                0xfau8, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa,
                0x00, 0x00
            ]
            .as_ref()
        )),
        token(b'L').map(|_| Either::Right(
            [
                0xfau8, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa,
                0x00, 0x00, 0xfa, 0x00, 0x00
            ]
            .as_ref()
        )),
        token(b'M').map(|_| Either::Right(
            [
                0xfau8, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa,
                0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00
            ]
            .as_ref()
        )),
        token(b'N').map(|_| Either::Right(
            [
                0xfau8, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa,
                0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00
            ]
            .as_ref()
        )),
        token(b'O').map(|_| Either::Right(
            [
                0xfau8, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa,
                0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00, 0xfa, 0x00, 0x00
            ]
            .as_ref()
        )),
        token(b'P').map(|_| Either::Right([0xfbu8, 0x80, 0x80].as_ref())),
        token(b'Q').map(|_| Either::Right([0xfcu8, 0x80, 0x80].as_ref())),
        token(b'R').map(|_| Either::Right([0xfdu8, 0x80, 0x80].as_ref())),
        token(b'S').map(|_| Either::Right([0x96u8, 0x69].as_ref())),
        token(b'T').map(|_| Either::Right([0x61u8, 0x01].as_ref())),
        token(b'U').map(|_| Either::Right([0xe1u8, 0x00, 0x00].as_ref())),
        token(b'Z').map(|_| Either::Left(0x00u8)),
        (hex_digit(), hex_digit()).map(|(u, l)| {
            let hex_to_u8 = |v: u8| match v {
                v if v >= b'0' && v <= b'9' => v - b'0',
                v if v >= b'A' && v <= b'F' => 10 + v - b'A',
                v if v >= b'a' && v <= b'f' => 10 + v - b'a',
                _ => unreachable!(),
            };
            let val = (hex_to_u8(u) << 4) | hex_to_u8(l);
            Either::Left(val)
        })
    )
    .message("while parsing MCC payload")
}

fn main() {
    println!("Hello, world!");
}

[Marwes]

Quick fix for this is to use https://docs.rs/combine/4.0.1/combine/fn.choice.html instead of the choice! macro. It compiles instantaneously when I changed that (it ends up creating a single, wide type instead of nested Or parsers).

[sdroege]

Thanks!

[sdroege]

This is btw the same function as in https://github.com/rust-lang/rust/issues/67454

[sdroege]

@Marwes Was this fixed in the meantime, and if so do you know which change(s) fixed it?

[Marwes]

https://github.com/rust-lang/rust/commit/18a366990335748d6020e306327048de9e4340c3 ought to have fixed it.

[sdroege]

Nice, I'll confirm with the next nightly that has this included. Thanks for the fast response!

[sdroege]

No, that still takes a long long time with 1.45.2. That commit is supposed to be in 1.42.0 and above.