Reduce duplication when passing context by value

Short

Is it possible to extract the fields of an object into a function's scope and pack them back into an object for returning? I am imagining a macro to make the following dance less verbose:

/// Just something to represent a non-copy value that we need.
#[derive(Debug)]
struct Resource;

/// Encapsulate some behaviour only available during render.
#[derive(Debug)]
struct RenderResource(Resource);

/// Variables we need to perform render.
struct RenderContext<'p> {
    a: Resource,
    b: &'p Resource, 
    c: &'p mut Resource,
}

fn render(context: RenderContext) -> RenderContext {
    let RenderContext { a, b, c } = context;
    let a = RenderResource(a);
    
    dbg!(&a, &b, &c);
    
    let a = a.0;
    RenderContext { a, b, c }
}

fn main() {
    let mut a = Resource;
    let b = &Resource;
    let mut c = &mut Resource;
    
    for _ in 0..2 {
        let context = render(RenderContext {
            a,
            b,
            c,
        });
        // Restore moved and mutably borrowed resources.
        a = context.a;
        // b = context.b; not necessary because &Resources is Copy.
        c = context.c;
    }
}

As you can see, I'm packing the context values before the call and unpacking after. Inside of the call I'm unpacking and repacking. Doing so adds a lot of code when you compare it to this version where render is inlined, especially when your context structs contain 10-20 variables and subsets of that are passed onto other functions with contexts of 5-10 variables and so forth.

/// Just something to represent a non-copy value that we need.
#[derive(Debug)]
struct Resource;

/// Encapsulate some behaviour only available during render.
#[derive(Debug)]
struct RenderResource(Resource);

/// Variables we need to perform render.
struct RenderContext<'p> {
    a: Resource,
    b: &'p Resource, 
    c: &'p mut Resource,
}

fn render(context: RenderContext) -> RenderContext {
    let RenderContext { a, b, c } = context;
    let a = RenderResource(a);
    
    dbg!(&a, &b, &c);
    
    let a = a.0;
    RenderContext { a, b, c }
}

fn main() {
    let mut a = Resource;
    let b = &Resource;
    let mut c = &mut Resource;
    
    for _ in 0..2 {
        let context = render(RenderContext {
            a,
            b,
            c,
        });
        // Restore moved and mutably borrowed resources.
        a = context.a;
        // b = context.b; not necessary because &Resources is Copy.
        c = context.c;
    }
}

I think unhygienic macros would help. I don't know if a procedural macro can help because I don't think they're supposed to have type information available to them but I hope I'm wrong.

Motivation

I'm writing a graphical application and my main function has grown to be a 1000+ lines. I've discovered that for profiling purpose I want to replay a set of inputs multiple times. To facilitate replaying I needed to separate "application-tied" resources (like textures and shaders) that should live for the entirety of the application from "run-tied" resources like the rng, scene state, keyboard input state and more. These two facts together encouraged me to try and refactor the application.

The current structure of the application looks like this:

fn main() {
    // Main resources. 100 lines

    for run_index in 0.. {
        // Run resources. 200 lines.

        for frame_index in 0.. {
            // Frame resources. 400 lines.
            process_events(/* ... */);
            simulate(/* ... */);
            render(/* ... */); 
        }
    }
}

I wanted to factor out the loop contents like so:

fn main() {
    // Main resources. 100 lines

    for run_index in 0.. {
        run(/* ... */)
    }
}

fn run(/* ... */) {
    // Run resources. 200 lines.

    for frame_index in 0.. {
        frame(/* ... */)
    }
}

fn frame(/* ... */) {
    // Frame resources. 400 lines.
    process_events(/* ... */);
    simulate(/* ... */);
    render(/* ... */); 
}

However, I failed to finish the refactor because the amount of code required to do the packing and unpacking was extreme and have a severe impact on the maintainability of the code. Making changes would require refactoring a lot of code that I think should be generated instead.

Working with context objects

So lets say we split our loop innards into their own functions. How do we make variables from outside the loop available? Usually you pass around some sort of "context" object with copies, references or mutable references to the required variables.

Lifetime proliferation

@matklad wrote a wonderful post on this topic illustrating what happens when you start to pass context objects with lifetimes around.

Basically if your loops are 5 levels deep, you might need to have a MyContext<'a, 'b, 'c, 'd, 'e> object to encapsulate that. If you ever need to add a loop in between or before the other loops, you need to change all descendant context objects everywhere. That is pretty painful because you may need to do mindless application wide changes to your parameters, even though you don't care what the exact lifetime is of the context variable you're using. You just need it to be available for the duration of the function.

Reborrowing

Another thing that comes up when working with context objects is "reborrowing" which @nikomatsakis detailed here along with 3 solutions to try.

Ownership

For some variables I want to have ownership. The reason is two-fold:

  1. I want to change the type to encapsulate some behavior or guard some invariant, but
  2. I don't want to create a chain of borrowed values because then we get lifetime proliferation and reaching the root requires many dereferences if the compiler doesn't inline your code.

@nikomatsakis has a post on dealing with not having ownership when you need it.

Conclusion

Many of you need to have run into these types of architectural problems when developing larger applications. I wonder what you've decided to do with your context. I know this post is fairly loose ended so if you have any words to share, they're more than welcome :slight_smile:.

I'm not sure I understand the question, but what about using getter methods to get access to just the fields you need?

Or making fields public so you can access them as struct.field instead of unpacking everything?

1 Like

Thanks for trying to answer my vague question. I was having trouble putting together a concise example because the nature of my question is architectural. It only shows at scale.

This is my main function, I will show you the MainContext and Context objects after.

fn main() {
    env_logger::init();

    let mut context = MainContext::new();

    let mut run_index = RunIndex::from_usize(0);
    let run_count = RunIndex::from_usize(match context.configuration.global.mode {
        ApplicationMode::Normal | ApplicationMode::Record => 1,
        ApplicationMode::Replay => context.configuration.replay.run_count,
    });

    while run_index < run_count {
        let mut context = Context::new(&mut context);

        context.profiling_context.begin_run(run_index);

        while context.running {
            context.render();
            context.process_events();
            context.simulate();
        }

        context.profiling_context.end_run(run_index);

        run_index.increment();
    }
}

The excerpt below is meant to illustrate the scale. Don't bother looking at the actual code, its more about the shape of it.

pub struct MainContext {
    pub current_dir: PathBuf,
    pub resource_dir: PathBuf,
    pub configuration_path: PathBuf,
    pub configuration: Configuration,
    pub events_loop: glutin::EventsLoop,
    pub gl_window: glutin::GlWindow,
    pub gl: gl::Gl,
    pub vr: Option<vr::Context>,
    pub fs_watcher: notify::RecommendedWatcher,
    pub fs_rx: mpsc::Receiver<notify::DebouncedEvent>,

    pub record_file: Option<io::BufWriter<fs::File>>,
    pub current: ::incremental::Current,
    pub shader_compiler: ShaderCompiler,
    pub profiling_context: ProfilingContext,
    pub replay_frame_events: Option<Vec<FrameEvents>>,
    pub initial_cameras: CameraMap<camera::Camera>,
    pub initial_win_dpi: f64,
    pub initial_win_size: glutin::dpi::PhysicalSize,

    // Text rendering.
    pub sans_serif: FontContext,
    pub monospace: FontContext,

    // Renderers
    pub depth_renderer: depth_renderer::Renderer,
    pub line_renderer: line_renderer::Renderer,
    pub basic_renderer: basic_renderer::Renderer,
    pub overlay_renderer: overlay_renderer::Renderer,
    pub cluster_renderer: cluster_renderer::Renderer,
    pub text_renderer: text_renderer::Renderer,
    pub cls_renderer: cls_renderer::Renderer,

    // More opengl resources...
    pub resources: Resources,

    // Per-frame resources
    pub main_parameters_vec: Vec<MainParameters>,
    pub camera_buffer_pool: BufferPool,
    pub light_resources_vec: Vec<light::LightResources>,
    pub light_params_vec: Vec<light::LightParameters>,
    pub cluster_resources_pool: ClusterResourcesPool,
    pub main_resources_pool: MainResourcesPool,
    pub point_lights: Vec<light::PointLight>,
}

impl MainContext {
    fn new() -> Self {
        let current_dir = std::env::current_dir().unwrap();
        let resource_dir: PathBuf = [current_dir.as_ref(), Path::new("resources")].into_iter().collect();
        let configuration_path = resource_dir.join(Configuration::DEFAULT_PATH);
        let configuration = Configuration::read(&configuration_path);

        let mut events_loop = glutin::EventsLoop::new();
        let gl_window = create_window(&mut events_loop, &configuration.window).unwrap();
        let gl = create_gl(&gl_window, &configuration.gl);
        let vr = vr::Context::new(vr::ApplicationType::Scene)
            .map_err(|error| {
                eprintln!("Failed to acquire context: {:?}", error);
            })
            .ok();

        let (fs_tx, fs_rx) = mpsc::channel();
        let mut fs_watcher = notify::watcher(fs_tx, time::Duration::from_millis(100)).unwrap();
        notify::Watcher::watch(&mut fs_watcher, &resource_dir, notify::RecursiveMode::Recursive).unwrap();

        let mut record_file = match configuration.global.mode {
            ApplicationMode::Record => Some(io::BufWriter::new(
                fs::File::create(&configuration.record.path).unwrap(),
            )),
            _ => None,
        };

        let mut replay_file = match configuration.global.mode {
            ApplicationMode::Replay => Some(io::BufReader::new(fs::File::open(&configuration.replay.path).unwrap())),
            _ => None,
        };

        let default_camera_transform = camera::CameraTransform {
            position: Point3::new(0.0, 1.0, 1.5),
            yaw: Rad(0.0),
            pitch: Rad(0.0),
            fovy: Deg(90.0).into(),
        };

        let mut initial_cameras = CameraMap::new(|key| camera::Camera {
            properties: match key {
                CameraKey::Main => configuration.main_camera,
                CameraKey::Debug => configuration.debug_camera,
            }
            .into(),
            transform: default_camera_transform,
        });

        // Load state.
        {
            let read_cameras = |file: &mut std::io::BufReader<std::fs::File>,
                                initial_cameras: &mut CameraMap<camera::Camera>| unsafe {
                for key in CameraKey::iter() {
                    file.read_exact(initial_cameras[key].value_as_bytes_mut())
                        .unwrap_or_else(|_| eprintln!("Failed to read state file."));
                }
            };

            match replay_file.as_mut() {
                Some(file) => {
                    read_cameras(file, &mut initial_cameras);
                }
                None => {
                    match fs::File::open("state.bin") {
                        Ok(file) => {
                            let mut file = io::BufReader::new(file);
                            read_cameras(&mut file, &mut initial_cameras);
                        }
                        Err(_) => {
                            // Whatever.
                        }
                    }
                }
            }
        }

        let replay_frame_events = replay_file.as_mut().map(|file| {
            let mut accumulator = Vec::new();

            while let Ok(events) = bincode::deserialize_from(&mut *file) {
                accumulator.push(events);
            }

            accumulator
        });

        if let Some(file) = record_file.as_mut() {
            for key in CameraKey::iter() {
                let camera = initial_cameras[key];
                file.write_all(camera.value_as_bytes()).unwrap();
            }
        }

        let sans_serif = FontContext::new(&gl, resource_dir.join("fonts/OpenSans-Regular.fnt"));
        let monospace = FontContext::new(&gl, resource_dir.join("fonts/RobotoMono-Regular.fnt"));

        let mut current = ::incremental::Current::new();

        let mut shader_compiler = ShaderCompiler::new(
            &current,
            shader_compiler::Variables {
                light_space: LightSpace::Wld,
                render_technique: RenderTechnique::Clustered,
                attenuation_mode: AttenuationMode::Interpolated,
                prefix_sum: configuration.prefix_sum,
                clustered_light_shading: configuration.clustered_light_shading,
            },
        );

        let mut rendering_context = RenderingContext {
            gl: &gl,
            resource_dir: &resource_dir,
            current: &mut current,
            shader_compiler: &mut shader_compiler,
        };

        let depth_renderer = depth_renderer::Renderer::new(&mut rendering_context);
        let line_renderer = line_renderer::Renderer::new(&mut rendering_context);
        let basic_renderer = basic_renderer::Renderer::new(&mut rendering_context);
        let overlay_renderer = overlay_renderer::Renderer::new(&mut rendering_context);
        let cluster_renderer = cluster_renderer::Renderer::new(&mut rendering_context);
        let text_renderer = text_renderer::Renderer::new(&mut rendering_context);
        let cls_renderer = cls_renderer::Renderer::new(&mut rendering_context);

        drop(rendering_context);

        let resources = resources::Resources::new(&gl, &resource_dir, &configuration);

        let initial_win_dpi = gl_window.get_hidpi_factor();
        let initial_win_size = gl_window.get_inner_size().unwrap().to_physical(initial_win_dpi);

        let profiling_context = ProfilingContext::new(&configuration.profiling);

        Self {
            current_dir,
            resource_dir,
            configuration_path,
            configuration,
            events_loop,
            gl_window,
            gl,
            vr,
            fs_watcher,
            fs_rx,
            record_file,
            current,
            shader_compiler,
            profiling_context,
            replay_frame_events,
            initial_cameras,
            initial_win_dpi,
            initial_win_size,
            sans_serif,
            monospace,
            depth_renderer,
            line_renderer,
            basic_renderer,
            overlay_renderer,
            cluster_renderer,
            text_renderer,
            cls_renderer,
            resources,
            camera_buffer_pool: BufferPool::new(),
            light_resources_vec: Vec::new(),
            light_params_vec: Vec::new(),
            main_parameters_vec: Vec::new(),
            cluster_resources_pool: ClusterResourcesPool::new(),
            main_resources_pool: MainResourcesPool::new(),
            point_lights: Vec::new(),
        }
    }
}

pub struct Context<'s> {
    // From MainContext
    pub current_dir: &'s Path,
    pub resource_dir: &'s Path,
    pub configuration_path: &'s Path,
    pub configuration: Configuration,
    pub events_loop: &'s mut glutin::EventsLoop,
    pub gl_window: &'s mut glutin::GlWindow,
    pub gl: &'s gl::Gl,
    pub vr: &'s mut Option<vr::Context>,
    pub fs_rx: &'s mut mpsc::Receiver<notify::DebouncedEvent>,

    pub record_file: &'s mut Option<io::BufWriter<fs::File>>,
    pub current: &'s mut ::incremental::Current,
    pub shader_compiler: &'s mut ShaderCompiler,
    pub profiling_context: &'s mut ProfilingContext,
    pub replay_frame_events: &'s Option<Vec<FrameEvents>>,

    // Text rendering.
    pub sans_serif: &'s mut FontContext,
    pub monospace: &'s mut FontContext,

    // Renderers
    pub depth_renderer: &'s mut depth_renderer::Renderer,
    pub line_renderer: &'s mut line_renderer::Renderer,
    pub basic_renderer: &'s mut basic_renderer::Renderer,
    pub overlay_renderer: &'s mut overlay_renderer::Renderer,
    pub cluster_renderer: &'s mut cluster_renderer::Renderer,
    pub text_renderer: &'s mut text_renderer::Renderer,
    pub cls_renderer: &'s mut cls_renderer::Renderer,

    // More opengl resources...
    pub resources: &'s mut Resources,

    // Per-frame resources
    pub main_parameters_vec: &'s mut Vec<MainParameters>,
    pub camera_buffer_pool: &'s mut BufferPool,
    pub light_resources_vec: &'s mut Vec<light::LightResources>,
    pub light_params_vec: &'s mut Vec<light::LightParameters>,
    pub cluster_resources_pool: &'s mut ClusterResourcesPool,
    pub main_resources_pool: &'s mut MainResourcesPool,
    pub point_lights: &'s mut Vec<light::PointLight>,

    pub rng: StdRng,
    pub overlay_textbox: TextBox,
    pub running: bool,
    pub paused: bool,
    pub focus: bool,
    pub tick: u64,
    pub event_index: usize,
    pub frame_index: FrameIndex,
    pub keyboard_state: KeyboardState,
    pub win_dpi: f64,
    pub win_size: glutin::dpi::PhysicalSize,
    pub clear_color: [f32; 3],
    pub window_mode: WindowMode,
    pub display_mode: u32,
    pub depth_prepass: bool,
    pub target_camera_key: CameraKey,
    pub transition_camera: camera::TransitionCamera,
    pub cameras: CameraMap<camera::SmoothCamera>,
    pub rain_drops: Vec<rain::Particle>,

    // Window.
    pub window_event_accumulator: WindowEventAccumulator,

    // FPS counter
    pub fps_average: filters::MovingAverageF32,
    pub last_frame_start: time::Instant,
}

impl<'s> Context<'s> {
    pub fn new(context: &'s mut MainContext) -> Self {
        let MainContext {
            ref current_dir,
            ref resource_dir,
            ref configuration_path,
            ref configuration,
            initial_win_dpi,
            initial_win_size,
            ref mut events_loop,
            ref mut gl_window,
            ref gl,
            ref mut vr,
            ref mut fs_rx,
            ref mut record_file,
            ref mut current,
            ref mut shader_compiler,
            ref mut profiling_context,
            ref replay_frame_events,
            ref initial_cameras,
            ref mut sans_serif,
            ref mut monospace,
            ref mut depth_renderer,
            ref mut line_renderer,
            ref mut basic_renderer,
            ref mut overlay_renderer,
            ref mut cluster_renderer,
            ref mut text_renderer,
            ref mut cls_renderer,
            ref mut resources,
            ref mut main_parameters_vec,
            ref mut camera_buffer_pool,
            ref mut light_resources_vec,
            ref mut light_params_vec,
            ref mut cluster_resources_pool,
            ref mut main_resources_pool,
            ref mut point_lights,
            ..
        } = *context;

        // Clone starting configuration.
        let configuration = configuration.clone();

        let transition_camera = camera::TransitionCamera {
            start_camera: initial_cameras.main,
            current_camera: initial_cameras.main,
            progress: 0.0,
        };
        let cameras = initial_cameras.map(|camera| camera::SmoothCamera {
            properties: camera.properties,
            current_transform: camera.transform,
            target_transform: camera.transform,
            smooth_enabled: true,
            current_smoothness: configuration.camera.maximum_smoothness,
            maximum_smoothness: configuration.camera.maximum_smoothness,
        });

        Context {
            current_dir,
            resource_dir,
            configuration_path,
            configuration,
            events_loop,
            gl_window,
            gl,
            vr,
            fs_rx,

            record_file,
            current,
            shader_compiler,
            profiling_context,
            replay_frame_events,

            // Text rendering.
            sans_serif,
            monospace,

            // Renderers
            depth_renderer,
            line_renderer,
            basic_renderer,
            overlay_renderer,
            cluster_renderer,
            text_renderer,
            cls_renderer,

            // More opengl resources...
            resources,

            // Per-frame resources
            main_parameters_vec,
            camera_buffer_pool,
            light_resources_vec,
            light_params_vec,
            cluster_resources_pool,
            main_resources_pool,
            point_lights,

            rng: SeedableRng::from_seed(SEED),
            running: true,
            paused: false,
            focus: false,
            tick: 0,
            event_index: 0,
            frame_index: FrameIndex::from_usize(0),
            keyboard_state: Default::default(),
            win_dpi: initial_win_dpi,
            win_size: initial_win_size,
            clear_color: [0.0; 3],
            window_mode: WindowMode::Main,
            display_mode: 1,
            depth_prepass: true,
            target_camera_key: CameraKey::Main,
            transition_camera,
            cameras,
            rain_drops: Vec::new(),
            window_event_accumulator: Default::default(),
            overlay_textbox: TextBox::new(
                13,
                10,
                initial_win_size.width as i32 - 26,
                initial_win_size.height as i32 - 20,
            ),
            fps_average: filters::MovingAverageF32::new(0.0),
            last_frame_start: Instant::now(),
        }
    }
}

It is hard to refactor the resources into separate structs because I use subsets of them in different places. Perhaps trying harder to group them is the right solution.

What I've tried so far was to flatten out the hierarchy so I'm flexible in how I bundle up the resources. While it works, it is an enormous amount of typing. I'd like to have a way (read: unhygienic macro or similar) to generate "packing" variables from the current function scope into an object and "unpacking" fields from an object back into the scope. Having the objects in the scope rather than an object helps when using closures so you don't have to create bindings for each field you use to move into the closure.

I wonder if you can take interfaces as arguments, instead. impl do_smth(&self, ctx: &impl SmthCtx); etc. This way you don't have to "repack" stuff and refactor code everywhere. You can pass the whole MainCtx, while the the SmthCtx guarantees that do_smth does not care about the details of what you pass to it. That should eliminate the churn. You can also use Into to eliminate conversion noise.

2 Likes

Thanks for your suggestion. I think it would work in some cases. However, when you're using a trait, you can't get multiple mutable references to different fields if you provide accessors per field. Also moving out and back into it becomes harder.

I'm going to think about this more and will construct a better and more concise example so we have something concrete. I realize now that this post is too vague.

1 Like

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.