respect api_version, bump api_version of asstet canister, upload chun…

…ks with 0 len

CHANGELOG.md

@@ -15,6 +15,15 @@ Valid settings are:
 The frontend canister sync now tries to batch multiple small content chunks into a single call using the `create_chunks` method added earlier.
 This should lead to significantly faster upload times for frontends with many small files.
 
+## Dependencies
+
+### Frontend canister
+
+Bumped `api_version` to `2` for the previous addition of `create_chunks` since the improved file sync relies on it.
+
+- Module hash: 9e4485d4358dd910aebcc025843547d05604cf28c6dc7c2cc2f8c76d083112e8
+- https://github.com/dfinity/sdk/pull/3947
+
 # 0.24.1
 
 ### feat: More PocketIC flags supported

src/canisters/frontend/ic-asset/src/batch_upload/plumbing.rs

@@ -54,6 +54,7 @@ type UploadQueue = Vec<(usize, Vec<u8>)>;
 pub(crate) struct ChunkUploader<'agent> {
     canister: Canister<'agent>,
     batch_id: Nat,
+    api_version: u16,
     chunks: Arc<AtomicUsize>,
     bytes: Arc<AtomicUsize>,
     // maps uploader_chunk_id to canister_chunk_id
@@ -62,10 +63,11 @@ pub(crate) struct ChunkUploader<'agent> {
 }
 
 impl<'agent> ChunkUploader<'agent> {
-    pub(crate) fn new(canister: Canister<'agent>, batch_id: Nat) -> Self {
+    pub(crate) fn new(canister: Canister<'agent>, api_version: u16, batch_id: Nat) -> Self {
         Self {
             canister,
             batch_id,
+            api_version,
             chunks: Arc::new(AtomicUsize::new(0)),
             bytes: Arc::new(AtomicUsize::new(0)),
             id_mapping: Arc::new(Mutex::new(BTreeMap::new())),
@@ -83,11 +85,12 @@ impl<'agent> ChunkUploader<'agent> {
     ) -> Result<usize, CreateChunkError> {
         let uploader_chunk_id = self.chunks.fetch_add(1, Ordering::SeqCst);
         self.bytes.fetch_add(contents.len(), Ordering::SeqCst);
-        if contents.len() == MAX_CHUNK_SIZE {
+        if contents.len() == MAX_CHUNK_SIZE || self.api_version < 2 {
             let canister_chunk_id =
                 create_chunk(&self.canister, &self.batch_id, contents, semaphores).await?;
             let mut map = self.id_mapping.lock().await;
             map.insert(uploader_chunk_id, canister_chunk_id);
+            println!("directly uploaded {uploader_chunk_id}");
             Ok(uploader_chunk_id)
         } else {
             self.add_to_upload_queue(uploader_chunk_id, contents).await;
@@ -97,7 +100,8 @@ impl<'agent> ChunkUploader<'agent> {
             //  - Tested with: `for i in $(seq 1 50); do dd if=/dev/urandom of="src/hello_frontend/assets/file_$i.bin" bs=$(shuf -i 1-2000000 -n 1) count=1; done && dfx deploy hello_frontend`
             //  - Result: Roughly 15% of batches under 90% full.
             // With other byte ranges (e.g. `shuf -i 1-3000000 -n 1`) stats improve significantly
-            self.upload_chunks(4 * MAX_CHUNK_SIZE, semaphores).await?;
+            self.upload_chunks(4 * MAX_CHUNK_SIZE, usize::MAX, semaphores)
+                .await?;
             Ok(uploader_chunk_id)
         }
     }
@@ -106,7 +110,8 @@ impl<'agent> ChunkUploader<'agent> {
         &self,
         semaphores: &Semaphores,
     ) -> Result<(), CreateChunkError> {
-        self.upload_chunks(0, semaphores).await
+        self.upload_chunks(0, 0, semaphores).await?;
+        Ok(())
     }
 
     pub(crate) fn bytes(&self) -> usize {
@@ -139,11 +144,12 @@ impl<'agent> ChunkUploader<'agent> {
     }
 
     /// Calls `upload_chunks` with batches of chunks from `self.upload_queue` until at most `max_retained_bytes`
-    /// bytes remain in the upload queue. Larger `max_retained_bytes` will lead to better batch fill rates
-    /// but also leave a larger memory footprint.
+    /// bytes and at most `max_retained_chunks` chunks remain in the upload queue. Larger values
+    /// will lead to better batch fill rates but also leave a larger memory footprint.
     async fn upload_chunks(
         &self,
         max_retained_bytes: usize,
+        max_retained_chunks: usize,
         semaphores: &Semaphores,
     ) -> Result<(), CreateChunkError> {
         let mut queue = self.upload_queue.lock().await;
@@ -154,6 +160,7 @@ impl<'agent> ChunkUploader<'agent> {
             .map(|(_, content)| content.len())
             .sum::<usize>()
             > max_retained_bytes
+            || queue.len() > max_retained_chunks
         {
             // Greedily fills batch with the largest chunk that fits
             queue.sort_unstable_by_key(|(_, content)| content.len());

src/canisters/frontend/ic-asset/src/sync.rs

@@ -45,6 +45,7 @@ const KNOWN_DIRECTORIES: [&str; 1] = [".well-known"];
 /// Sets the contents of the asset canister to the contents of a directory, including deleting old assets.
 pub async fn upload_content_and_assemble_sync_operations(
     canister: &Canister<'_>,
+    canister_api_version: u16,
     dirs: &[&Path],
     no_delete: bool,
     logger: &Logger,
@@ -74,7 +75,8 @@ pub async fn upload_content_and_assemble_sync_operations(
         "Staging contents of new and changed assets in batch {}:", batch_id
     );
 
-    let chunk_uploader = ChunkUploader::new(canister.clone(), batch_id.clone());
+    let chunk_uploader =
+        ChunkUploader::new(canister.clone(), canister_api_version, batch_id.clone());
 
     let project_assets = make_project_assets(
         Some(&chunk_uploader),
@@ -123,9 +125,15 @@ pub async fn sync(
     no_delete: bool,
     logger: &Logger,
 ) -> Result<(), SyncError> {
-    let commit_batch_args =
-        upload_content_and_assemble_sync_operations(canister, dirs, no_delete, logger).await?;
     let canister_api_version = api_version(canister).await;
+    let commit_batch_args = upload_content_and_assemble_sync_operations(
+        canister,
+        canister_api_version,
+        dirs,
+        no_delete,
+        logger,
+    )
+    .await?;
     debug!(logger, "Canister API version: {canister_api_version}. ic-asset API version: {BATCH_UPLOAD_API_VERSION}");
     info!(logger, "Committing batch.");
     match canister_api_version {
@@ -198,7 +206,15 @@ pub async fn prepare_sync_for_proposal(
     dirs: &[&Path],
     logger: &Logger,
 ) -> Result<(Nat, ByteBuf), PrepareSyncForProposalError> {
-    let arg = upload_content_and_assemble_sync_operations(canister, dirs, false, logger).await?;
+    let canister_api_version = api_version(canister).await;
+    let arg = upload_content_and_assemble_sync_operations(
+        canister,
+        canister_api_version,
+        dirs,
+        false,
+        logger,
+    )
+    .await?;
     let arg = sort_batch_operations(arg);
     let batch_id = arg.batch_id.clone();
 

src/canisters/frontend/ic-asset/src/upload.rs

@@ -39,10 +39,12 @@ pub async fn upload(
     info!(logger, "Starting batch.");
 
     let batch_id = create_batch(canister).await.map_err(CreateBatchFailed)?;
+    let canister_api_version = api_version(canister).await;
 
     info!(logger, "Staging contents of new and changed assets:");
 
-    let chunk_upload_target = ChunkUploader::new(canister.clone(), batch_id.clone());
+    let chunk_upload_target =
+        ChunkUploader::new(canister.clone(), canister_api_version, batch_id.clone());
 
     let project_assets = make_project_assets(
         Some(&chunk_upload_target),

src/canisters/frontend/ic-certified-assets/src/lib.rs

@@ -35,7 +35,7 @@ thread_local! {
 #[query]
 #[candid_method(query)]
 fn api_version() -> u16 {
-    1
+    2
 }
 
 #[update(guard = "is_manager_or_controller")]