The err_into method seems slow

[dnaka91]

I saw that winnow 0.5.12 does some performance improvements and went playing around. Realized that err_into() is somewhat slow, even though it just got some adjustments for inlining.

Originally I forgot about err_into, and made my own map_err function.

Now when trying to switch back to err_into, I realized that the performance degraded. Roughly things are between 4-8% slower when using err_into over my own map_err.

This is my map_err implementation, which is very similar to the err_into implementation (except having a closure to define the conversion instead of relying on the From trait):

trait ParserExt {
    fn map_err<G, I, O, E, E2>(self, map: G) -> MapErr<Self, G, I, O, E, E2>
    where
        G: Fn(E) -> E2,
        Self: Parser<I, O, E> + Sized;
}

impl<T> ParserExt for T {
    fn map_err<G, I, O, E, E2>(self, map: G) -> MapErr<Self, G, I, O, E, E2>
    where
        G: Fn(E) -> E2,
        Self: Parser<I, O, E> + Sized,
    {
        MapErr::new(self, map)
    }
}

pub struct MapErr<F, G, I, O, E, E2>
where
    F: Parser<I, O, E>,
    G: Fn(E) -> E2,
{
    parser: F,
    map: G,
    i: PhantomData<I>,
    o: PhantomData<O>,
    e: PhantomData<E>,
    e2: PhantomData<E2>,
}

impl<F, G, I, O, E, E2> MapErr<F, G, I, O, E, E2>
where
    F: Parser<I, O, E>,
    G: Fn(E) -> E2,
{
    pub(crate) fn new(parser: F, map: G) -> Self {
        Self {
            parser,
            map,
            i: PhantomData,
            o: PhantomData,
            e: PhantomData,
            e2: PhantomData,
        }
    }
}

impl<F, G, I, O, E, E2> Parser<I, O, E2> for MapErr<F, G, I, O, E, E2>
where
    F: Parser<I, O, E>,
    G: Fn(E) -> E2,
{
    #[inline]
    fn parse_next(&mut self, i: &mut I) -> PResult<O, E2> {
        match self.parser.parse_next(i) {
            Ok(o) => Ok(o),
            Err(e) => Err(e.map(|e| (self.map)(e))),
        }
    }
}

Compared to the current MapInto implementation:

impl<F, I, O, E, E2> Parser<I, O, E2> for ErrInto<F, I, O, E, E2>
where
    F: Parser<I, O, E>,
    E: Into<E2>,
{
    #[inline]
    fn parse_next(&mut self, i: &mut I) -> PResult<O, E2> {
        match self.parser.parse_next(i) {
            Ok(ok) => Ok(ok),
            Err(ErrMode::Backtrack(e)) => Err(ErrMode::Backtrack(e.into())),
            Err(ErrMode::Cut(e)) => Err(ErrMode::Cut(e.into())),
            Err(ErrMode::Incomplete(e)) => Err(ErrMode::Incomplete(e)),
        }
    }
}

This is pretty much the same, except the MapInto version even does the ErrMode conversion manually... So I would have expected it to be equivalent in speed or even faster 🤔

[epage]

To really say much about how these compare, I'd need a reproduction case of the slowdown. I find a lot of these performance issues are finicky in how they show up.

[dnaka91]

I noticed that especially #[inline] can have surprising effects.

Generally, one would assume adding those will improve performance at the cost of binary size. But in some cases, I noticed that removing inline attributes will even make performance better.

As you said, it's probably hard to compare, and the performance impact depends highly on what parsers are being built. I could imagine that it's slow for me, but may be faster in other parsers.

Just put this out here as these seemingly equivalent looking implementations seem to have some difference performance.

[epage]

As you said, it's probably hard to compare, and the performance impact depends highly on what parsers are being built. I could imagine that it's slow for me, but may be faster in other parsers.

I'd like to not dismiss things for that. While there are trade offs where we have to decide who we optimize for, there are other cases where there can be tweaks on the callers side of no one has really been impacted by a performance issue yet. For example, I'm working on switching gitoxide to winnow and the performance has been pretty bad and I fully think its worth investigating why until we have someone more concrete to point to.

[dnaka91]

I was able to reproduce the performance difference in a minimal setup. Will publish it as a repo tomorrow so that we might investigate further on this.

While building the setup, I noticed that the performance is mostly visible when parsing larger amounts like 1000 times the same element, and when only parsing a few lines it has no noticeable impact.

[dnaka91]

So after quite a long while of testing this again and again over time, I didn't get a really reliable way of reproducing the difference. Sometimes there is a difference (in the microsecond level), and then sometimes it's about the same time.

Especially, recent patch releases positively affected the ErrInto to a level where it doesn't really have any noticeable difference anymore.

I'll go ahead and mark this as answer because that difference disappeared. Also, great work on the recent performance tuning, really happy to see this dedication to make it super fast.