Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bigger chunk sizes than 4KB? #710

Closed
pwmarcz opened this issue Jul 23, 2022 · 2 comments
Closed

Bigger chunk sizes than 4KB? #710

pwmarcz opened this issue Jul 23, 2022 · 2 comments

Comments

@pwmarcz
Copy link
Contributor

pwmarcz commented Jul 23, 2022

It seems that for SerenityOS, loading 4KB "sectors" over the network is a massive bottleneck. Loading data in bigger chunks, and caching it, improves the experience drastically:

Setup Chunk size Time
copy.sh/v86, Serenity browser launch 4 KB 7m 55s (blank window appears 2m 30s in)
local, Serenity browser launch 8 KB 5m 32s (blank window appears 2m 10s in)
local, Serenity browser launch 4 MB 37s (blank window appears 20s in)
local, Serenity boot 8 KB 7m 15s
local, Serenity boot 4 MB 1m 05s

I also tried this setup with Windows 98, and applications (My Computer, Solitaire) start noticeably faster, but the difference is not as big.

Experimental setup

When testing locally, I configured v86 to also use the k.copy.sh server (by going to http://localhost:8000/?profile=serenity&cdn=//k.copy.sh/ and enabling CORS in browser).

I ran all tests in Firefox, with cache disabled.

Here's the patch:

diff --git a/src/browser/lib.js b/src/browser/lib.js
index 26933318..b4516475 100644
--- a/src/browser/lib.js
+++ b/src/browser/lib.js
@@ -404,11 +404,11 @@ var ASYNC_SAFE = false;
             {
                 block.set(written_block, i * this.block_size);
             }
-            //else
-            //{
-            //    var cached = this.loaded_blocks[start_block + i] = new Uint8Array(this.block_size);
-            //    cached.set(block.subarray(i * this.block_size, (i + 1) * this.block_size));
-            //}
+            else
+            {
+               var cached = this.loaded_blocks[start_block + i] = new Uint8Array(this.block_size);
+               cached.set(block.subarray(i * this.block_size, (i + 1) * this.block_size));
+            }
         }
     };

@@ -580,15 +580,21 @@ var ASYNC_SAFE = false;
         }
         else
         {
-            const part_filename = this.basename + "-" + offset + "-" + (offset + len) + this.extension;
+            const chunk_size = 4 * 1024 * 1024;
+            const start = offset - (offset % chunk_size);
+            // TODO: if (offset + len) is aligned, this ends up reading one chunk too many
+            // (so chunk_size of 4KB actually ends up reading 8KB)
+            let end = (offset + len + chunk_size) - ((offset + len) % chunk_size);
+            const read_len = end - start;
+            const part_filename = this.basename + "-" + start + "-" + (start + read_len) + this.extension;

             v86util.load_file(part_filename, {
                 done: function done(buffer)
                 {
-                    dbg_assert(buffer.byteLength === len);
-                    var block = new Uint8Array(buffer);
-                    this.handle_read(offset, len, block);
-                    fn(block);
+                    const blocks = new Uint8Array(buffer);
+                    this.handle_read(start, buffer.byteLength, blocks);
+                    const tmp_blocks = blocks.subarray(offset - start, offset + len - start);
+                    fn(tmp_blocks);
                 }.bind(this),
             });
         }
diff --git a/src/browser/main.js b/src/browser/main.js
index 3995dd47..b60ee446 100644
--- a/src/browser/main.js
+++ b/src/browser/main.js
@@ -218,7 +218,7 @@
                     "url": host + "serenity-v2.img",
                     "async": true,
                     "size": 700448768,
-                    use_parts: !ON_LOCALHOST,
+                    use_parts: true,
                 },
                 memory_size: 512 * 1024 * 1024,
                 state: { url: host + "serenity_state-v3.bin.zst", },
@copy
Copy link
Owner

copy commented Jul 24, 2022

Interesting analysis.

At the moment the size depends on the request the guest OS sends to the IDE controller. Some OSes seem to request the exact range they need from the disk (e.g. Haiku, Windows 2000), so they wouldn't benefit from this change. There is already a fixed_chunk_size argument that could be used.

@copy
Copy link
Owner

copy commented Jul 29, 2022

I stand corrected: The improvements are significant for all OSes. I've measured the number of requests and the total downloaded bytes depending on various block sizes, and assigned block sizes between 256k and 1m. If you go much bigger, the downloaded size goes up significantly (because blocks are dowloaded that the OS didn't need).

Thanks for the suggestion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants