Enhance OSDK performance by reusing existing base, bundle and build

Cargo.toml

@@ -38,7 +38,10 @@ exclude = [
     "kernel/libs/comp-sys/component-macro",
     "kernel/libs/comp-sys/controlled",
     "osdk",
+    # The `base` and `test-base` crates are auto-generated by OSDK and ultimately built by Cargo
+    # during `cargo osdk run` and `cargo osdk test` commands
     "target/osdk/base",
+    "target/osdk/test-base",
 ]
 
 # Cargo only looks at the profile settings 

Makefile

@@ -153,6 +153,10 @@ OSDK_CRATES := \
 	kernel/libs/aster-util \
 	kernel/libs/aster-bigtcp
 
+# OSDK dependencies
+OSDK_SRC_FILES := \
+	$(shell find osdk/Cargo.toml osdk/Cargo.lock osdk/src -type f)
+
 .PHONY: all
 all: build
 
@@ -165,10 +169,9 @@ install_osdk:
 	@# dependencies to `crates.io`.
 	@OSDK_LOCAL_DEV=1 cargo install cargo-osdk --path osdk
 
-# This will install OSDK if it is not already installed
+# This will install and update OSDK automatically
-# To update OSDK, we need to run `install_osdk` manually
+$(CARGO_OSDK): $(OSDK_SRC_FILES)
-$(CARGO_OSDK):
+	@$(MAKE) --no-print-directory install_osdk
-	@make --no-print-directory install_osdk
 
 .PHONY: check_osdk
 check_osdk:
@@ -182,7 +185,7 @@ test_osdk:
 
 .PHONY: initramfs
 initramfs:
-	@make --no-print-directory -C test
+	@$(MAKE) --no-print-directory -C test
 
 .PHONY: build
 build: initramfs $(CARGO_OSDK)
@@ -215,7 +218,7 @@ gdb_server: initramfs $(CARGO_OSDK)
 	@cargo osdk run $(CARGO_OSDK_ARGS) --gdb-server wait-client,vscode,addr=:$(GDB_TCP_PORT)
 
 .PHONY: gdb_client
-gdb_client: $(CARGO_OSDK)
+gdb_client: initramfs $(CARGO_OSDK)
 	@cargo osdk debug $(CARGO_OSDK_ARGS) --remote :$(GDB_TCP_PORT)
 
 .PHONY: profile_server
@@ -223,7 +226,7 @@ profile_server: initramfs $(CARGO_OSDK)
 	@cargo osdk run $(CARGO_OSDK_ARGS) --gdb-server addr=:$(GDB_TCP_PORT)
 
 .PHONY: profile_client
-profile_client: $(CARGO_OSDK)
+profile_client: initramfs $(CARGO_OSDK)
 	@cargo osdk profile $(CARGO_OSDK_ARGS) --remote :$(GDB_TCP_PORT) \
 		--samples $(GDB_PROFILE_COUNT) --interval $(GDB_PROFILE_INTERVAL) --format $(GDB_PROFILE_FORMAT)
 
@@ -257,7 +260,7 @@ docs: $(CARGO_OSDK)
 .PHONY: format
 format:
 	@./tools/format_all.sh
-	@make --no-print-directory -C test format
+	@$(MAKE) --no-print-directory -C test format
 
 .PHONY: check
 check: initramfs $(CARGO_OSDK)
@@ -279,7 +282,7 @@ check: initramfs $(CARGO_OSDK)
 		echo "Checking $$dir"; \
 		(cd $$dir && cargo osdk clippy -- -- -D warnings) || exit 1; \
 	done
-	@make --no-print-directory -C test check
+	@$(MAKE) --no-print-directory -C test check
 	@typos
 
 .PHONY: clean
@@ -291,6 +294,6 @@ clean:
 	@echo "Cleaning up documentation target files"
 	@cd docs && mdbook clean
 	@echo "Cleaning up test target files"
-	@make --no-print-directory -C test clean
+	@$(MAKE) --no-print-directory -C test clean
 	@echo "Uninstalling OSDK"
 	@rm -f $(CARGO_OSDK)

osdk/src/base_crate/mod.rs

@@ -5,12 +5,45 @@
 
 use std::{
     fs,
+    io::{Read, Result},
     path::{Path, PathBuf},
     str::FromStr,
 };
 
 use crate::util::get_cargo_metadata;
 
+/// Compares two files byte-by-byte to check if they are identical.
+/// Returns `Ok(true)` if files are identical, `Ok(false)` if they are different, or `Err` if any I/O operation fails.
+fn are_files_identical(file1: &PathBuf, file2: &PathBuf) -> Result<bool> {
+    // Check file size first
+    let metadata1 = fs::metadata(file1)?;
+    let metadata2 = fs::metadata(file2)?;
+
+    if metadata1.len() != metadata2.len() {
+        return Ok(false); // Different sizes, not identical
+    }
+
+    // Compare file contents byte-by-byte
+    let mut file1 = fs::File::open(file1)?;
+    let mut file2 = fs::File::open(file2)?;
+
+    let mut buffer1 = [0u8; 4096];
+    let mut buffer2 = [0u8; 4096];
+
+    loop {
+        let bytes_read1 = file1.read(&mut buffer1)?;
+        let bytes_read2 = file2.read(&mut buffer2)?;
+
+        if bytes_read1 != bytes_read2 || buffer1[..bytes_read1] != buffer2[..bytes_read1] {
+            return Ok(false); // Files are different
+        }
+
+        if bytes_read1 == 0 {
+            return Ok(true); // End of both files, identical
+        }
+    }
+}
+
 /// Create a new base crate that will be built by cargo.
 ///
 /// The dependencies of the base crate will be the target crate. If
@@ -21,6 +54,43 @@ pub fn new_base_crate(
     dep_crate_name: &str,
     dep_crate_path: impl AsRef<Path>,
     link_unit_test_runner: bool,
+) {
+    // Check if the existing crate base is reusable. Crate bases for ktest are never reusable.
+    if !base_crate_path.as_ref().ends_with("test-base") && base_crate_path.as_ref().exists() {
+        let base_crate_tmp_path = base_crate_path.as_ref().join("tmp");
+        do_new_base_crate(
+            &base_crate_tmp_path,
+            dep_crate_name,
+            &dep_crate_path,
+            link_unit_test_runner,
+        );
+        let cargo_result = are_files_identical(
+            &base_crate_path.as_ref().join("Cargo.toml"),
+            &base_crate_tmp_path.join("Cargo.toml"),
+        );
+        let main_rs_result = are_files_identical(
+            &base_crate_path.as_ref().join("src").join("main.rs"),
+            &base_crate_tmp_path.join("src").join("main.rs"),
+        );
+        std::fs::remove_dir_all(&base_crate_tmp_path).unwrap();
+        if cargo_result.is_ok_and(|res| res) && main_rs_result.is_ok_and(|res| res) {
+            info!("Reusing existing base crate");
+            return;
+        }
+    }
+    do_new_base_crate(
+        base_crate_path,
+        dep_crate_name,
+        dep_crate_path,
+        link_unit_test_runner,
+    );
+}
+
+fn do_new_base_crate(
+    base_crate_path: impl AsRef<Path>,
+    dep_crate_name: &str,
+    dep_crate_path: impl AsRef<Path>,
+    link_unit_test_runner: bool,
 ) {
     let workspace_root = {
         let meta = get_cargo_metadata(None::<&str>, None::<&[&str]>).unwrap();

osdk/src/bundle/bin.rs

@@ -83,12 +83,11 @@ impl AsterBin {
         self.stripped
     }
 
-    /// Move the binary to the `base` directory and convert the path to a relative path.
+    /// Copy the binary to the `base` directory and convert the path to a relative path.
-    pub fn move_to(self, base: impl AsRef<Path>) -> Self {
+    pub fn copy_to(self, base: impl AsRef<Path>) -> Self {
         let file_name = self.path.file_name().unwrap();
         let copied_path = base.as_ref().join(file_name);
         fs::copy(&self.path, copied_path).unwrap();
-        fs::remove_file(&self.path).unwrap();
         Self {
             path: PathBuf::from(file_name),
             arch: self.arch,

osdk/src/bundle/file.rs

@@ -53,7 +53,7 @@ impl Initramfs {
         }
     }
 
-    /// Move the initramfs to the `base` directory and convert the path to a relative path.
+    /// Copy the initramfs to the `base` directory and convert the path to a relative path.
     pub fn copy_to(self, base: impl AsRef<Path>) -> Self {
         let name = self.path.file_name().unwrap();
         let dest = base.as_ref().join(name);

osdk/src/bundle/mod.rs

@@ -291,7 +291,7 @@ impl Bundle {
         if self.manifest.vm_image.is_some() {
             panic!("vm_image already exists");
         }
-        self.manifest.vm_image = Some(vm_image.move_to(&self.path));
+        self.manifest.vm_image = Some(vm_image.copy_to(&self.path));
         self.write_manifest_to_fs();
     }
 
@@ -300,7 +300,7 @@ impl Bundle {
         if self.manifest.aster_bin.is_some() {
             panic!("aster_bin already exists");
         }
-        self.manifest.aster_bin = Some(aster_bin.move_to(&self.path));
+        self.manifest.aster_bin = Some(aster_bin.copy_to(&self.path));
         self.write_manifest_to_fs();
     }
 

osdk/src/bundle/vm_image.rs

@@ -59,8 +59,8 @@ impl AsterVmImage {
         &self.typ
     }
 
-    /// Move the binary to the `base` directory and convert the path to a relative path.
+    /// Copy the binary to the `base` directory and convert the path to a relative path.
-    pub fn move_to(self, base: impl AsRef<Path>) -> Self {
+    pub fn copy_to(self, base: impl AsRef<Path>) -> Self {
         let file_name = self.path.file_name().unwrap();
         let copied_path = base.as_ref().join(file_name);
         fs::copy(&self.path, copied_path).unwrap();

osdk/src/commands/build/mod.rs

@@ -8,7 +8,7 @@ use std::{
     ffi::OsString,
     path::{Path, PathBuf},
     process,
-    time::{Duration, SystemTime},
+    time::SystemTime,
 };
 
 use bin::make_elf_for_qemu;
@@ -19,6 +19,7 @@ use crate::{
     base_crate::new_base_crate,
     bundle::{
         bin::{AsterBin, AsterBinType, AsterElfMeta},
+        file::BundleFile,
         Bundle,
     },
     cli::BuildArgs,
@@ -88,6 +89,31 @@ pub fn create_base_and_cached_build(
     bundle
 }
 
+fn get_reusable_existing_bundle(
+    bundle_path: impl AsRef<Path>,
+    config: &Config,
+    action: ActionChoice,
+) -> Option<Bundle> {
+    let existing_bundle = Bundle::load(&bundle_path);
+    let Some(existing_bundle) = existing_bundle else {
+        info!("Building a new bundle: No cached bundle found or validation of the existing bundle failed");
+        return None;
+    };
+    if let Err(e) = existing_bundle.can_run_with_config(config, action) {
+        info!("Building a new bundle: {}", e);
+        return None;
+    }
+    let workspace_root = {
+        let meta = get_cargo_metadata(None::<&str>, None::<&[&str]>).unwrap();
+        PathBuf::from(meta.get("workspace_root").unwrap().as_str().unwrap())
+    };
+    if existing_bundle.last_modified_time() < get_last_modified_time(&workspace_root) {
+        info!("Building a new bundle: workspace_root has been updated");
+        return None;
+    }
+    Some(existing_bundle)
+}
+
 /// If the source is not since modified and the last build is recent, we can reuse the existing bundle.
 pub fn do_cached_build(
     bundle_path: impl AsRef<Path>,
@@ -97,54 +123,6 @@ pub fn do_cached_build(
     action: ActionChoice,
     rustflags: &[&str],
 ) -> Bundle {
-    let build_a_new_one = || {
-        do_build(
-            &bundle_path,
-            &osdk_output_directory,
-            &cargo_target_directory,
-            config,
-            action,
-            rustflags,
-        )
-    };
-
-    let existing_bundle = Bundle::load(&bundle_path);
-    let Some(existing_bundle) = existing_bundle else {
-        return build_a_new_one();
-    };
-    if existing_bundle.can_run_with_config(config, action).is_err() {
-        return build_a_new_one();
-    }
-    let Ok(built_since) = SystemTime::now().duration_since(existing_bundle.last_modified_time())
-    else {
-        return build_a_new_one();
-    };
-    if built_since > Duration::from_secs(600) {
-        return build_a_new_one();
-    }
-    let workspace_root = {
-        let meta = get_cargo_metadata(None::<&str>, None::<&[&str]>).unwrap();
-        PathBuf::from(meta.get("workspace_root").unwrap().as_str().unwrap())
-    };
-    if get_last_modified_time(workspace_root) < existing_bundle.last_modified_time() {
-        return existing_bundle;
-    }
-    build_a_new_one()
-}
-
-pub fn do_build(
-    bundle_path: impl AsRef<Path>,
-    osdk_output_directory: impl AsRef<Path>,
-    cargo_target_directory: impl AsRef<Path>,
-    config: &Config,
-    action: ActionChoice,
-    rustflags: &[&str],
-) -> Bundle {
-    if bundle_path.as_ref().exists() {
-        std::fs::remove_dir_all(&bundle_path).unwrap();
-    }
-    let mut bundle = Bundle::new(&bundle_path, config, action);
-
     let (build, boot) = match action {
         ActionChoice::Run => (&config.run.build, &config.run.boot),
         ActionChoice::Test => (&config.test.build, &config.test.boot),
@@ -160,6 +138,21 @@ pub fn do_build(
         rustflags,
     );
 
+    // Check the existing bundle's reusability
+    if let Some(existing_bundle) = get_reusable_existing_bundle(&bundle_path, config, action) {
+        if get_last_modified_time(aster_elf.path()) < existing_bundle.last_modified_time() {
+            info!("Reusing existing bundle: aster_elf is unchanged");
+            return existing_bundle;
+        }
+    }
+
+    // Build a new bundle
+    info!("Building a new bundle");
+    if bundle_path.as_ref().exists() {
+        std::fs::remove_dir_all(&bundle_path).unwrap();
+    }
+    let mut bundle = Bundle::new(&bundle_path, config, action);
+
     match boot.method {
         BootMethod::GrubRescueIso | BootMethod::GrubQcow2 => {
             info!("Building boot device image");
@@ -274,6 +267,9 @@ fn build_kernel_elf(
 }
 
 fn get_last_modified_time(path: impl AsRef<Path>) -> SystemTime {
+    if path.as_ref().is_file() {
+        return path.as_ref().metadata().unwrap().modified().unwrap();
+    }
     let mut last_modified = SystemTime::UNIX_EPOCH;
     for entry in std::fs::read_dir(path).unwrap() {
         let entry = entry.unwrap();

osdk/src/commands/new/mod.rs

@@ -47,9 +47,10 @@ fn add_manifest_dependencies(cargo_metadata: &serde_json::Value, crate_name: &st
     fs::write(manifest_path, content).unwrap();
 }
 
-// Add `target/osdk/base` to `exclude` array of the workspace manifest
+// Add `target/osdk/base` and `target/osdk/test-base` to `exclude` array of the workspace manifest
 fn exclude_osdk_base(metadata: &serde_json::Value) {
-    let osdk_base_path = "target/osdk/base";
+    let osdk_run_base_path = "target/osdk/base";
+    let osdk_test_base_path = "target/osdk/test-base";
 
     let workspace_manifest_path = {
         let workspace_root = metadata.get("workspace_root").unwrap().as_str().unwrap();
@@ -64,17 +65,25 @@ fn exclude_osdk_base(metadata: &serde_json::Value) {
 
         if let Some(exclude) = workspace.get_mut("exclude") {
             let exclude = exclude.as_array_mut().unwrap();
-            if exclude.contains(&toml::Value::String(osdk_base_path.to_string())) {
+            if exclude.contains(&toml::Value::String(osdk_run_base_path.to_string()))
+                || exclude.contains(&toml::Value::String(osdk_test_base_path.to_string()))
+            {
                 return;
             }
 
-            exclude.push(toml::Value::String(osdk_base_path.to_string()));
+            exclude.push(toml::Value::String(osdk_run_base_path.to_string()));
+            exclude.push(toml::Value::String(osdk_test_base_path.to_string()));
         } else {
-            let exclude = vec![toml::Value::String(osdk_base_path.to_string())];
+            let exclude = vec![
+                toml::Value::String(osdk_run_base_path.to_string()),
+                toml::Value::String(osdk_test_base_path.to_string()),
+            ];
             workspace.insert("exclude".to_string(), toml::Value::Array(exclude));
         }
     } else {
-        let exclude = toml::Table::from_str(r#"exclude = ["target/osdk/base"]"#).unwrap();
+        let exclude =
+            toml::Table::from_str(r#"exclude = ["target/osdk/base", "target/osdk/test-base"]"#)
+                .unwrap();
         manifest_toml.insert("workspace".to_string(), toml::Value::Table(exclude));
     }
 

osdk/src/commands/test.rs

@@ -26,7 +26,8 @@ pub fn test_current_crate(config: &Config, args: &TestArgs) {
     let current_crate = get_current_crate_info();
     let cargo_target_directory = get_target_directory();
     let osdk_output_directory = cargo_target_directory.join(DEFAULT_TARGET_RELPATH);
-    let target_crate_dir = osdk_output_directory.join("base");
+    // Use a different name for better separation and reusability of `run` and `test`
+    let target_crate_dir = osdk_output_directory.join("test-base");
 
     // A special case is that we use OSDK to test the OSDK test runner crate
     // itself. We check it by name.

osdk/src/config/scheme/qemu.rs

@@ -34,7 +34,7 @@ pub struct QemuScheme {
     pub path: Option<PathBuf>,
 }
 
-#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+#[derive(Debug, Clone, Eq, Serialize, Deserialize)]
 pub struct Qemu {
     pub args: String,
     /// This finalized config has a unorthodox `Option` because
@@ -55,6 +55,20 @@ impl Default for Qemu {
     }
 }
 
+// Implements `PartialEq` for `Qemu`, comparing `args` while ignoring numeric characters
+// (random ports), and comparing other fields (`bootdev_append_options` and `path`) normally.
+impl PartialEq for Qemu {
+    fn eq(&self, other: &Self) -> bool {
+        fn strip_numbers(input: &str) -> String {
+            input.chars().filter(|c| !c.is_numeric()).collect()
+        }
+
+        strip_numbers(&self.args) == strip_numbers(&other.args)
+            && self.bootdev_append_options == other.bootdev_append_options
+            && self.path == other.path
+    }
+}
+
 impl Qemu {
     pub fn apply_qemu_args(&mut self, args: &Vec<String>) {
         let mut joined = split_to_kv_array(&self.args);

osdk/tests/util/mod.rs

@@ -57,7 +57,10 @@ pub fn create_workspace(workspace_name: &str, members: &[&str]) {
             .map(|member| toml::Value::String(member.to_string()))
             .collect();
 
-        let exclude = toml::Value::Array(vec![toml::Value::String("target/osdk/base".to_string())]);
+        let exclude = toml::Value::Array(vec![
+            toml::Value::String("target/osdk/base".to_string()),
+            toml::Value::String("target/osdk/test-base".to_string()),
+        ]);
 
         table.insert("members".to_string(), toml::Value::Array(members));
         table.insert("exclude".to_string(), exclude);