Re-arranging functions for inlining

I think it is correct to say that the compiler will not inline "parts" of a function, so it seems to me that in some cases you might want to split a function up so that part of it can be inlined. Here is an example of what I mean:

impl<'a, K, V> Iterator for Range<'a, K, V> {
    type Item = (&'a K, &'a V);
    #[inline]
    fn next(&mut self) -> Option<Self::Item> {
        if let Some(f) = &mut self.fwd_leaf {
            if let Some(x) = f.next() {
                return Some(x);
            }
            self.fwd_leaf = None;
        }
        self.next_inner()
    }
}

from lib.rs - source

Here I have renamed next to next_inner, so I can inline the "start" of it for performance reasons ( so the most common case will execute inline, and it will only need to do the more complicated other bits quite rarely ).

Is this a common/valid technique?

Yeah, for example: Option::unwrap

I think this is more about being able to mark the less common branch as #[cold] than for inlining reasons. I don't think not inlining will improve performance in most cases, but at least #[cold] leaves that up to the compiler.

1 Like

Thanks. Incidentally, here next_inner has a loop where the first part could be skipped when it is initially called, so unless the compiler is clever enough to spot this, I need something like a "GOTO" to jump into the middle of the loop, see the START comments I have added below. Is there a way to do that? Maybe the I could re-write the function somehow, but it isn't obvious to me how this can be done.

Edit: Oh, hmm, I have a feeling I can omit the initial code entirely, I don't think it can ever get there... , oh, no, it can after StealResult::CT(ct) => self.push_tree(ct, false). But maybe then I can move it to there. Hmm!

    fn next_inner(&mut self) -> Option<(&'a K, &'a V)> {
        /// GOTO START
        loop {
            if let Some(f) = &mut self.fwd_leaf {
                if let Some(x) = f.next() {
                    return Some(x);
                }
                self.fwd_leaf = None;
            } /* START : */ else if let Some(s) = self.fwd_stk.last_mut() {
                if let Some(kv) = s.v.next() {
                    if let Some(ct) = s.c.next() {
                        self.push_tree(ct, false);
                    }
                    return Some(kv);
                }
                self.fwd_stk.pop();
            } else {
                match self.steal_bck() {
                    StealResult::KV(kv) => return Some(kv),
                    StealResult::CT(ct) => self.push_tree(ct, false),
                    StealResult::Nothing => {
                        if let Some(f) = &mut self.bck_leaf {
                            if let Some(x) = f.next() {
                                return Some(x);
                            }
                            self.bck_leaf = None;
                        }
                        return None;
                    }
                }
            }
        }
    }

The guaranteed-to-work solution is to break it into two parts.

fn next_inner(&mut self) -> Option<(&'a K, &'a V)> {
    if let Some(s) = self.fwd_stk.last_mut() {
        if let Some(kv) = s.v.next() {
            if let Some(ct) = s.c.next() {
                self.push_tree(ct, false);
            }
            return Some(kv);
        }
        self.fwd_stk.pop();
    } else {
        match self.steal_bck() {
            StealResult::KV(kv) => return Some(kv),
            StealResult::CT(ct) => self.push_tree(ct, false),
            StealResult::Nothing => {
                if let Some(f) = &mut self.bck_leaf {
                    if let Some(x) = f.next() {
                        return Some(x);
                    }
                    self.bck_leaf = None;
                }
                return None;
            }
        }
    }

    loop {
        if let Some(f) = &mut self.fwd_leaf {
            if let Some(x) = f.next() {
                return Some(x);
            }
            self.fwd_leaf = None;
        } else if let Some(s) = self.fwd_stk.last_mut() {
            if let Some(kv) = s.v.next() {
                if let Some(ct) = s.c.next() {
                    self.push_tree(ct, false);
                }
                return Some(kv);
            }
            self.fwd_stk.pop();
        } else {
            match self.steal_bck() {
                StealResult::KV(kv) => return Some(kv),
                StealResult::CT(ct) => self.push_tree(ct, false),
                StealResult::Nothing => {
                    if let Some(f) = &mut self.bck_leaf {
                        if let Some(x) = f.next() {
                            return Some(x);
                        }
                        self.bck_leaf = None;
                    }
                    return None;
                }
            }
        }
    }
}

It looks like you don't actually need else if the first time though? The only possible outcomes of the first if is a return or immediately failing on the second loop.

if let Some(f) = &mut self.fwd_leaf {
    if let Some(x) = f.next() {
        return Some(x);
    }
    self.fwd_leaf = None;
}
if let Some(s) = self.fwd_stk.last_mut() {
    if let Some(kv) = s.v.next() {
        if let Some(ct) = s.c.next() {
            self.push_tree(ct, false);
        }
        return Some(kv);
    }
    self.fwd_stk.pop();
} else {
    match self.steal_bck() {
        StealResult::KV(kv) => return Some(kv),
        StealResult::CT(ct) => self.push_tree(ct, false),
        StealResult::Nothing => {
            if let Some(f) = &mut self.bck_leaf {
                if let Some(x) = f.next() {
                    return Some(x);
                }
                self.bck_leaf = None;
            }
            return None;
        }
    }
}

Which means that you can just move it to the end of the loop.

if let Some(s) = self.fwd_stk.last_mut() {
    if let Some(kv) = s.v.next() {
        if let Some(ct) = s.c.next() {
            self.push_tree(ct, false);
        }
        return Some(kv);
    }
    self.fwd_stk.pop();
} else {
    match self.steal_bck() {
        StealResult::KV(kv) => return Some(kv),
        StealResult::CT(ct) => self.push_tree(ct, false),
        StealResult::Nothing => {
            if let Some(f) = &mut self.bck_leaf {
                if let Some(x) = f.next() {
                    return Some(x);
                }
                self.bck_leaf = None;
            }
            return None;
        }
    }
}
if let Some(f) = &mut self.fwd_leaf {
    if let Some(x) = f.next() {
        return Some(x);
    }
    self.fwd_leaf = None;
}
1 Like

You are right, but I think the best place to move it is just after this executes, as that is the only place it can be true. Sorry, I am "coding out loud" here, and not thinking too well either!

StealResult::CT(ct) => self.push_tree(ct, false),
1 Like

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.