Address code review feedback

src/data_processing.rs

@@ -69,17 +69,19 @@ pub fn generate_world_with_options(
 
     println!("{} Processing data...", "[4/7]".bold());
 
-    // Build highway connectivity map once before processing
-    let highway_connectivity = highways::build_highway_connectivity_map(&elements);
-
     // Set ground reference in the editor to enable elevation-aware block placement
     editor.set_ground(Arc::clone(&ground));
 
     println!("{} Processing terrain...", "[5/7]".bold());
     emit_gui_progress_update(25.0, "Processing terrain...");
 
-    // Pre-compute all flood fills in parallel for better CPU utilization
-    let mut flood_fill_cache = FloodFillCache::precompute(&elements, args.timeout.as_ref());
+    // Run both precomputations concurrently using rayon::join
+    // This overlaps highway connectivity map building with flood fill computation
+    let timeout_ref = args.timeout.as_ref();
+    let (highway_connectivity, mut flood_fill_cache) = rayon::join(
+        || highways::build_highway_connectivity_map(&elements),
+        || FloodFillCache::precompute(&elements, timeout_ref),
+    );
     println!("Pre-computed {} flood fills", flood_fill_cache.way_count());
 
     // Process data

src/element_processing/highways.rs

@@ -5,6 +5,7 @@ use crate::coordinate_system::cartesian::XZPoint;
 use crate::floodfill_cache::FloodFillCache;
 use crate::osm_parser::{ProcessedElement, ProcessedWay};
 use crate::world_editor::WorldEditor;
+use rayon::prelude::*;
 use std::collections::HashMap;
 
 /// Type alias for highway connectivity map
@@ -28,39 +29,41 @@ pub fn generate_highways(
 }
 
 /// Build a connectivity map for highway endpoints to determine where slopes are needed.
+/// Uses parallel processing for better performance on large element sets.
 pub fn build_highway_connectivity_map(elements: &[ProcessedElement]) -> HighwayConnectivityMap {
-    let mut connectivity_map: HashMap<(i32, i32), Vec<i32>> = HashMap::new();
+    // Parallel map phase: extract connectivity data from each highway element
+    let partial_maps: Vec<Vec<((i32, i32), i32)>> = elements
+        .par_iter()
+        .filter_map(|element| {
+            if let ProcessedElement::Way(way) = element {
+                if way.tags.contains_key("highway") && !way.nodes.is_empty() {
+                    let layer_value = way
+                        .tags
+                        .get("layer")
+                        .and_then(|layer| layer.parse::<i32>().ok())
+                        .unwrap_or(0);
 
-    for element in elements {
-        if let ProcessedElement::Way(way) = element {
-            if way.tags.contains_key("highway") {
-                let layer_value = way
-                    .tags
-                    .get("layer")
-                    .and_then(|layer| layer.parse::<i32>().ok())
-                    .unwrap_or(0);
+                    // Treat negative layers as ground level (0) for connectivity
+                    let layer_value = if layer_value < 0 { 0 } else { layer_value };
 
-                // Treat negative layers as ground level (0) for connectivity
-                let layer_value = if layer_value < 0 { 0 } else { layer_value };
-
-                // Add connectivity for start and end nodes
-                if !way.nodes.is_empty() {
                     let start_node = &way.nodes[0];
                     let end_node = &way.nodes[way.nodes.len() - 1];
 
                     let start_coord = (start_node.x, start_node.z);
                     let end_coord = (end_node.x, end_node.z);
 
-                    connectivity_map
-                        .entry(start_coord)
-                        .or_default()
-                        .push(layer_value);
-                    connectivity_map
-                        .entry(end_coord)
-                        .or_default()
-                        .push(layer_value);
+                    return Some(vec![(start_coord, layer_value), (end_coord, layer_value)]);
                 }
             }
+            None
+        })
+        .collect();
+
+    // Sequential reduce phase: merge all partial results into final map
+    let mut connectivity_map: HashMap<(i32, i32), Vec<i32>> = HashMap::new();
+    for entries in partial_maps {
+        for (coord, layer) in entries {
+            connectivity_map.entry(coord).or_default().push(layer);
         }
     }
 

src/world_editor/java.rs

@@ -3,7 +3,7 @@
 //! This module handles saving worlds in the Java Edition Anvil (.mca) format.
 //! Supports streaming mode for memory-efficient saving of large worlds.
 
-use super::common::{Chunk, ChunkToModify, Section};
+use super::common::{Chunk, ChunkToModify, RegionToModify, Section};
 use super::WorldEditor;
 use crate::block_definitions::GRASS_BLOCK;
 use crate::progress::emit_gui_progress_update;
@@ -12,73 +12,21 @@ use fastanvil::Region;
 use fastnbt::Value;
 use fnv::FnvHashMap;
 use indicatif::{ProgressBar, ProgressStyle};
+use rayon::prelude::*;
 use std::collections::HashMap;
 use std::fs::File;
 use std::io::Write;
+use std::path::Path;
+use std::sync::atomic::{AtomicU64, Ordering};
 
 #[cfg(feature = "gui")]
 use crate::telemetry::{send_log, LogLevel};
 
 impl<'a> WorldEditor<'a> {
-    /// Creates a region file for the given region coordinates.
-    pub(super) fn create_region(&self, region_x: i32, region_z: i32) -> Region<File> {
-        let region_dir = self.world_dir.join("region");
-        let out_path = region_dir.join(format!("r.{}.{}.mca", region_x, region_z));
-
-        // Ensure region directory exists before creating region files
-        std::fs::create_dir_all(&region_dir).expect("Failed to create region directory");
-
-        const REGION_TEMPLATE: &[u8] = include_bytes!("../../assets/minecraft/region.template");
-
-        let mut region_file: File = File::options()
-            .read(true)
-            .write(true)
-            .create(true)
-            .truncate(true)
-            .open(&out_path)
-            .expect("Failed to open region file");
-
-        region_file
-            .write_all(REGION_TEMPLATE)
-            .expect("Could not write region template");
-
-        Region::from_stream(region_file).expect("Failed to load region")
-    }
-
-    /// Helper function to create a base chunk with grass blocks at Y -62
-    pub(super) fn create_base_chunk(abs_chunk_x: i32, abs_chunk_z: i32) -> (Vec<u8>, bool) {
-        let mut chunk = ChunkToModify::default();
-
-        // Fill the bottom layer with grass blocks at Y -62
-        for x in 0..16 {
-            for z in 0..16 {
-                chunk.set_block(x, -62, z, GRASS_BLOCK);
-            }
-        }
-
-        // Prepare chunk data
-        let chunk_data = Chunk {
-            sections: chunk.sections().collect(),
-            x_pos: abs_chunk_x,
-            z_pos: abs_chunk_z,
-            is_light_on: 0,
-            other: chunk.other,
-        };
-
-        // Create the Level wrapper
-        let level_data = create_level_wrapper(&chunk_data);
-
-        // Serialize the chunk with Level wrapper
-        let mut ser_buffer = Vec::with_capacity(8192);
-        fastnbt::to_writer(&mut ser_buffer, &level_data).unwrap();
-
-        (ser_buffer, true)
-    }
-
     /// Saves the world in Java Edition Anvil format.
     ///
-    /// Uses streaming mode: saves regions one at a time and releases memory after each,
-    /// significantly reducing peak memory usage for large worlds.
+    /// Uses parallel processing: saves multiple regions concurrently for faster I/O,
+    /// while still releasing memory after each region is processed.
     pub(super) fn save_java(&mut self) {
         println!("{} Saving world...", "[7/7]".bold());
         emit_gui_progress_update(90.0, "Saving world...");
@@ -92,6 +40,12 @@ impl<'a> WorldEditor<'a> {
         }
 
         let total_regions = self.world.regions.len() as u64;
+
+        // Early return if no regions to save (prevents division by zero)
+        if total_regions == 0 {
+            return;
+        }
+
         let save_pb = ProgressBar::new(total_regions);
         save_pb.set_style(
             ProgressStyle::default_bar()
@@ -102,155 +56,208 @@ impl<'a> WorldEditor<'a> {
                 .progress_chars("█▓░"),
         );
 
-        // Streaming mode: Process regions sequentially and release memory after each.
-        // This significantly reduces peak memory for large worlds (100+ regions).
-        // For small worlds, the overhead is negligible.
-        let mut regions_processed: u64 = 0;
+        // Ensure region directory exists before parallel processing
+        let region_dir = self.world_dir.join("region");
+        std::fs::create_dir_all(&region_dir).expect("Failed to create region directory");
 
-        // Collect region keys first to allow draining
-        let region_keys: Vec<(i32, i32)> = self.world.regions.keys().copied().collect();
+        // Drain all regions from memory into a Vec for parallel processing
+        let regions_to_save: Vec<((i32, i32), super::common::RegionToModify)> =
+            self.world.regions.drain().collect();
 
-        for (region_x, region_z) in region_keys {
-            // Remove region from memory - this is the key to memory savings
-            if let Some(region_to_modify) = self.world.regions.remove(&(region_x, region_z)) {
-                self.save_single_region(region_x, region_z, &region_to_modify);
+        // Track progress atomically across threads
+        let regions_processed = AtomicU64::new(0);
+        let world_dir = self.world_dir.clone();
+
+        // Process regions in parallel, each region file is independent
+        regions_to_save
+            .into_par_iter()
+            .for_each(|((region_x, region_z), region_to_modify)| {
+                // Save this region (creates its own file handle)
+                save_region_to_file(&world_dir, region_x, region_z, &region_to_modify);
+
+                // Update progress atomically
+                let processed = regions_processed.fetch_add(1, Ordering::Relaxed) + 1;
+                save_pb.inc(1);
+
+                // Emit GUI progress update periodically
+                let update_interval = (total_regions / 10).max(1);
+                if processed.is_multiple_of(update_interval) || processed == total_regions {
+                    let progress = 90.0 + (processed as f64 / total_regions as f64) * 9.0;
+                    emit_gui_progress_update(progress, "Saving world...");
+                }
 
                 // Region memory is freed when region_to_modify goes out of scope here
-            }
-
-            regions_processed += 1;
-
-            // Update progress at regular intervals
-            let update_interval = (total_regions / 10).max(1);
-            if regions_processed.is_multiple_of(update_interval)
-                || regions_processed == total_regions
-            {
-                let progress = 90.0 + (regions_processed as f64 / total_regions as f64) * 9.0;
-                emit_gui_progress_update(progress, "Saving world...");
-            }
-
-            save_pb.inc(1);
-        }
+            });
 
         save_pb.finish();
     }
+}
 
-    /// Saves a single region to disk.
-    ///
-    /// This is extracted to allow streaming mode to save and release regions one at a time.
-    fn save_single_region(
-        &self,
-        region_x: i32,
-        region_z: i32,
-        region_to_modify: &super::common::RegionToModify,
-    ) {
-        let mut region = self.create_region(region_x, region_z);
-        let mut ser_buffer = Vec::with_capacity(8192);
+/// Saves a single region to a file (thread-safe, for parallel processing).
+///
+/// This is a standalone function that can be called from parallel threads
+/// since it only needs the world directory path, not a reference to WorldEditor.
+fn save_region_to_file(
+    world_dir: &Path,
+    region_x: i32,
+    region_z: i32,
+    region_to_modify: &RegionToModify,
+) {
+    // Create region file
+    let region_dir = world_dir.join("region");
+    let out_path = region_dir.join(format!("r.{}.{}.mca", region_x, region_z));
 
-        for (&(chunk_x, chunk_z), chunk_to_modify) in &region_to_modify.chunks {
-            if !chunk_to_modify.sections.is_empty() || !chunk_to_modify.other.is_empty() {
-                // Read existing chunk data if it exists
-                let existing_data = region
-                    .read_chunk(chunk_x as usize, chunk_z as usize)
-                    .unwrap()
-                    .unwrap_or_default();
+    const REGION_TEMPLATE: &[u8] = include_bytes!("../../assets/minecraft/region.template");
 
-                // Parse existing chunk or create new one
-                let mut chunk: Chunk = if !existing_data.is_empty() {
-                    fastnbt::from_bytes(&existing_data).unwrap()
+    let mut region_file: File = File::options()
+        .read(true)
+        .write(true)
+        .create(true)
+        .truncate(true)
+        .open(&out_path)
+        .expect("Failed to open region file");
+
+    region_file
+        .write_all(REGION_TEMPLATE)
+        .expect("Could not write region template");
+
+    let mut region = Region::from_stream(region_file).expect("Failed to load region");
+    let mut ser_buffer = Vec::with_capacity(8192);
+
+    // First pass: write modified chunks
+    for (&(chunk_x, chunk_z), chunk_to_modify) in &region_to_modify.chunks {
+        if !chunk_to_modify.sections.is_empty() || !chunk_to_modify.other.is_empty() {
+            // Read existing chunk data if it exists
+            let existing_data = region
+                .read_chunk(chunk_x as usize, chunk_z as usize)
+                .unwrap()
+                .unwrap_or_default();
+
+            // Parse existing chunk or create new one
+            let mut chunk: Chunk = if !existing_data.is_empty() {
+                fastnbt::from_bytes(&existing_data).unwrap()
+            } else {
+                Chunk {
+                    sections: Vec::new(),
+                    x_pos: chunk_x + (region_x * 32),
+                    z_pos: chunk_z + (region_z * 32),
+                    is_light_on: 0,
+                    other: FnvHashMap::default(),
+                }
+            };
+
+            // Update sections while preserving existing data
+            let new_sections: Vec<Section> = chunk_to_modify.sections().collect();
+            for new_section in new_sections {
+                if let Some(existing_section) =
+                    chunk.sections.iter_mut().find(|s| s.y == new_section.y)
+                {
+                    // Merge block states
+                    existing_section.block_states.palette = new_section.block_states.palette;
+                    existing_section.block_states.data = new_section.block_states.data;
                 } else {
-                    Chunk {
-                        sections: Vec::new(),
-                        x_pos: chunk_x + (region_x * 32),
-                        z_pos: chunk_z + (region_z * 32),
-                        is_light_on: 0,
-                        other: FnvHashMap::default(),
-                    }
-                };
+                    // Add new section if it doesn't exist
+                    chunk.sections.push(new_section);
+                }
+            }
 
-                // Update sections while preserving existing data
-                let new_sections: Vec<Section> = chunk_to_modify.sections().collect();
-                for new_section in new_sections {
-                    if let Some(existing_section) =
-                        chunk.sections.iter_mut().find(|s| s.y == new_section.y)
+            // Preserve existing block entities and merge with new ones
+            if let Some(existing_entities) = chunk.other.get_mut("block_entities") {
+                if let Some(new_entities) = chunk_to_modify.other.get("block_entities") {
+                    if let (Value::List(existing), Value::List(new)) =
+                        (existing_entities, new_entities)
                     {
-                        // Merge block states
-                        existing_section.block_states.palette = new_section.block_states.palette;
-                        existing_section.block_states.data = new_section.block_states.data;
-                    } else {
-                        // Add new section if it doesn't exist
-                        chunk.sections.push(new_section);
+                        // Remove old entities that are replaced by new ones
+                        existing.retain(|e| {
+                            if let Value::Compound(map) = e {
+                                let (x, y, z) = get_entity_coords(map);
+                                !new.iter().any(|new_e| {
+                                    if let Value::Compound(new_map) = new_e {
+                                        let (nx, ny, nz) = get_entity_coords(new_map);
+                                        x == nx && y == ny && z == nz
+                                    } else {
+                                        false
+                                    }
+                                })
+                            } else {
+                                true
+                            }
+                        });
+                        // Add new entities
+                        existing.extend(new.clone());
                     }
                 }
-
-                // Preserve existing block entities and merge with new ones
-                if let Some(existing_entities) = chunk.other.get_mut("block_entities") {
-                    if let Some(new_entities) = chunk_to_modify.other.get("block_entities") {
-                        if let (Value::List(existing), Value::List(new)) =
-                            (existing_entities, new_entities)
-                        {
-                            // Remove old entities that are replaced by new ones
-                            existing.retain(|e| {
-                                if let Value::Compound(map) = e {
-                                    let (x, y, z) = get_entity_coords(map);
-                                    !new.iter().any(|new_e| {
-                                        if let Value::Compound(new_map) = new_e {
-                                            let (nx, ny, nz) = get_entity_coords(new_map);
-                                            x == nx && y == ny && z == nz
-                                        } else {
-                                            false
-                                        }
-                                    })
-                                } else {
-                                    true
-                                }
-                            });
-                            // Add new entities
-                            existing.extend(new.clone());
-                        }
-                    }
-                } else {
-                    // If no existing entities, just add the new ones
-                    if let Some(new_entities) = chunk_to_modify.other.get("block_entities") {
-                        chunk
-                            .other
-                            .insert("block_entities".to_string(), new_entities.clone());
-                    }
+            } else {
+                // If no existing entities, just add the new ones
+                if let Some(new_entities) = chunk_to_modify.other.get("block_entities") {
+                    chunk
+                        .other
+                        .insert("block_entities".to_string(), new_entities.clone());
                 }
+            }
 
-                // Update chunk coordinates and flags
-                chunk.x_pos = chunk_x + (region_x * 32);
-                chunk.z_pos = chunk_z + (region_z * 32);
+            // Update chunk coordinates and flags
+            chunk.x_pos = chunk_x + (region_x * 32);
+            chunk.z_pos = chunk_z + (region_z * 32);
 
-                // Create Level wrapper and save
-                let level_data = create_level_wrapper(&chunk);
-                ser_buffer.clear();
-                fastnbt::to_writer(&mut ser_buffer, &level_data).unwrap();
+            // Create Level wrapper and save
+            let level_data = create_level_wrapper(&chunk);
+            ser_buffer.clear();
+            fastnbt::to_writer(&mut ser_buffer, &level_data).unwrap();
+            region
+                .write_chunk(chunk_x as usize, chunk_z as usize, &ser_buffer)
+                .unwrap();
+        }
+    }
+
+    // Second pass: ensure all chunks exist (create base chunks for empty slots)
+    for chunk_x in 0..32 {
+        for chunk_z in 0..32 {
+            let abs_chunk_x = chunk_x + (region_x * 32);
+            let abs_chunk_z = chunk_z + (region_z * 32);
+
+            // Check if chunk exists in our modifications
+            let chunk_exists = region_to_modify.chunks.contains_key(&(chunk_x, chunk_z));
+
+            // If chunk doesn't exist, create it with base layer
+            if !chunk_exists {
+                let (ser_buffer, _) = create_base_chunk(abs_chunk_x, abs_chunk_z);
                 region
                     .write_chunk(chunk_x as usize, chunk_z as usize, &ser_buffer)
                     .unwrap();
             }
         }
+    }
+}
 
-        // Second pass: ensure all chunks exist
-        for chunk_x in 0..32 {
-            for chunk_z in 0..32 {
-                let abs_chunk_x = chunk_x + (region_x * 32);
-                let abs_chunk_z = chunk_z + (region_z * 32);
+/// Helper function to create a base chunk with grass blocks at Y -62 (standalone version)
+fn create_base_chunk(abs_chunk_x: i32, abs_chunk_z: i32) -> (Vec<u8>, bool) {
+    let mut chunk = ChunkToModify::default();
 
-                // Check if chunk exists in our modifications
-                let chunk_exists = region_to_modify.chunks.contains_key(&(chunk_x, chunk_z));
-
-                // If chunk doesn't exist, create it with base layer
-                if !chunk_exists {
-                    let (ser_buffer, _) = Self::create_base_chunk(abs_chunk_x, abs_chunk_z);
-                    region
-                        .write_chunk(chunk_x as usize, chunk_z as usize, &ser_buffer)
-                        .unwrap();
-                }
-            }
+    // Fill the bottom layer with grass blocks at Y -62
+    for x in 0..16 {
+        for z in 0..16 {
+            chunk.set_block(x, -62, z, GRASS_BLOCK);
         }
     }
+
+    // Prepare chunk data
+    let chunk_data = Chunk {
+        sections: chunk.sections().collect(),
+        x_pos: abs_chunk_x,
+        z_pos: abs_chunk_z,
+        is_light_on: 0,
+        other: chunk.other,
+    };
+
+    // Create the Level wrapper
+    let level_data = create_level_wrapper(&chunk_data);
+
+    // Serialize the chunk with Level wrapper
+    let mut ser_buffer = Vec::with_capacity(8192);
+    fastnbt::to_writer(&mut ser_buffer, &level_data).unwrap();
+
+    (ser_buffer, true)
 }
 
 /// Helper function to get entity coordinates