memory management - Can I avoid cloning Strings by holding a reference instead?

Question

I have a data structure in which I am providing a wrapper around a read buffer to automatically handle repeating statements in the readout.

This is done by storing an internal state of how many repeats are left and the line to be repeated.

use std::fs::File;
use std::path::Path;
use std::io::BufReader;
use std::io::prelude::*;
use std::io::Error;
use std::num::NonZeroU32;
use std::mem;

pub struct Reader {
    handle: BufReader<File>,
    repeat_state: Option<(NonZeroU32, String)>,
}

impl Reader {
    pub fn new<P: AsRef<Path>>(path: P) -> Result<Reader, Error> {
        let file = File::open(path)?;
        let handle = BufReader::new(file);

        Ok(Reader {
            handle,
            repeat_state: None,
        })
    }

    /// get next line, respecting repeat instructions
    pub fn next_line(&mut self) -> Option<String> {
        if self.repeat_state.is_some() {
            let (repeats_left, last_line) = mem::replace(&mut self.repeat_state, None).unwrap();

            self.repeat_state = NonZeroU32::new(repeats_left.get() - 1)
                .map(|repeats_left| (repeats_left, last_line.clone()));

            Some(last_line)
        } else {
            let mut line = String::new();
            if self.handle.read_line(&mut line).is_err() || line.is_empty() {
                return None
            }

            if line.starts_with("repeat ") {
                let repeats: Option<u32> = line.chars().skip(7)
                    .take_while(|c| c.is_numeric())
                    .collect::<String>().parse().ok();

                self.repeat_state = repeats
                    .and_then(|repeats| NonZeroU32::new(repeats - 1))
                    .map(|repeats_left| (repeats_left, line.clone()))
            }

            Some(line)
        }
    }
}

#[test]
fn test_next_line() {
    let source = "
line one
repeat 2    line two and line three
line four
repeat 11   lines 5-15
line 16
line 17
last line (18)
    ".trim();
    let mut input = File::create("file.txt").unwrap();
    write!(input, "{}", source);


    let mut read = Reader::new("file.txt").unwrap();
    assert_eq!(
        read.next_line(),
        Some("line one
".to_string())
    );
    assert_eq!(
        read.next_line(),
        Some("repeat 2    line two and line three
".to_string())
    );
    assert_eq!(
        read.next_line(),
        Some("repeat 2    line two and line three
".to_string())
    );
    assert_eq!(
        read.next_line(),
        Some("line four
".to_string())
    );

    for _ in 5..=15 {
        assert_eq!(
            read.next_line(),
            Some("repeat 11   lines 5-15
".to_string())
        );
    }

    assert_eq!(
        read.next_line(),
        Some("line 16
".to_string())
    );
    assert_eq!(
        read.next_line(),
        Some("line 17
".to_string())
    );
    assert_eq!(
        read.next_line(),
        Some("last line (18)".to_string())
    );
}

Playground

The problem is that I have to clone the held repeated value every time in order to both hold onto it and return it. I want to avoid these costly clones by returning (and maybe storing) a &str. I've tried several things, but was unable to get it to work:

• Storing String, returning &str: "does not live long enough" lifetime errors
• Storing &str, returning &str: same lifetime errors
• Cow<&str>
• Box<&str>

These clones are the bottleneck of my program at the moment, according to the CodeXL time-based sampling profiler after building in release mode with debug info. Now, my program is plenty fast as it is, but I'm wondering if there is a way of avoiding them.

Answer 1

You can avoid cloning the strings by wrapping them in an Rc and cloning that instead. Cloning an Rc is cheap since it consists of incrementing a counter:

pub struct Reader {
    handle: BufReader<File>,
    repeat_state: Option<(NonZeroU32, Rc<String>)>,
}

impl Reader {
    pub fn new<P: AsRef<Path>>(path: P) -> Result<Reader, Error> {
        let file = File::open(path)?;
        let handle = BufReader::new(file);

        Ok(Reader {
            handle,
            repeat_state: None,
        })
    }

    /// get next line, respecting repeat instructions
    pub fn next_line(&mut self) -> Option<Rc<String>> {
        if self.repeat_state.is_some() {
            let (repeats_left, last_line) = mem::replace(&mut self.repeat_state, None).unwrap();

            self.repeat_state = NonZeroU32::new(repeats_left.get() - 1)
                .map(|repeats_left| (repeats_left, last_line.clone()));

            Some(last_line)
        } else {
            let mut line = Rc::new (String::new());
            if self.handle.read_line(Rc::make_mut (&mut line)).is_err() || line.is_empty() {
                return None
            }

            if line.starts_with("repeat ") {
                let repeats: Option<u32> = line.chars().skip(7)
                    .take_while(|c| c.is_numeric())
                    .collect::<String>().parse().ok();

                self.repeat_state = repeats
                    .and_then(|repeats| NonZeroU32::new(repeats - 1))
                    .map(|repeats_left| (repeats_left, line.clone()))
            }

            Some(line)
        }
    }
}

playground

Note that Rc cannot be shared between multiple threads. If you want to share the strings between threads, you can use Arc instead.