Merge pull request #27 from bldl/optimizing-section

mathiasness · web-flow · commit e621ea9edb76 · 2024-12-05T12:44:50.000+01:00
imported updated version of optimization-section
diff --git a/upsert-tutorial/index.html b/upsert-tutorial/index.html
@@ -84,7 +84,7 @@ <h2>What&#39;s Covered in This Tutorial?</h2>
 
                 <!-- Collapsible Sections for Detailed Topics -->
                 <section class="collapsible" id="proposal-overview">
-                    <summary><h2>The `Map.prototype.upsert` proposal</h2></summary>
+                    <h2>The `Map.prototype.upsert` proposal</h2>
                     <p><strong>What is it?</strong></p>
                     <p><code>Map.prototype.upsert</code> is a new method for <a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map"><code>Map</code>-object</a> in JavaScript™. The operation simplifies the process of inserting or updating key-value pairs in the <code>Map</code>: it checks for existence of a key and then either <code>insert</code>s or <code>update</code>s a key-value pair.  </p>
                     <p>This proposal is also intended for <a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakMap"><code>WeakMap</code></a>, which shares a similar API structure. The behavior of <code>upsert</code> in <code>WeakMap</code> would follow the same principles, with the primary difference being the behavior of keys in <code>WeakMap</code>, as they are held weakly and can be garbage collected when no other references to them exist. In this tutorial however, we will focus on the <code>Map</code>-implementation.</p>
@@ -1318,26 +1318,29 @@ <h2>Writing the Specification in Ecmarkup</h2>
                 <section class="collapsible" id="optimization">
                     <h2>Optimization</h2>
                     <div class="content-body">
-                        <p>A proposal goes through several <a href="https://www.proposals.es/stages" target="_blank">stages</a> before it becomes a part of the ECMAScript® language.
-                            Every new feature introduces complexity, which can affect the performance of the SpiderMonkey engine.
-                            Therefore optimization becomes crucial when designing and implementing these features.
-                            In our case there is especially one line which could use some optimization:</p>
-                        <pre><code>4. For each Record { [[Key]], [[Value]] } e that is an element of entries, do</code></pre>
-                        <p>  As of right now it is implemented like this:</p>
-                        <pre><code class="language-js">for (var e of allowContentIter(entries)) {
+                        <p>When a new feature is being added to the language, we must consider the complexity it introduces - this can affect the performance of the SpiderMonkey engine.
+                            Therefore, optimization becomes crucial when designing and implementing proposals.
+                            In our case, step 4 of the specification </p>
+                        <pre><code class="language-lua">4. For each Record { [[Key]], [[Value]] } e that is an element of entries, do
+</code></pre>
+                        <p>  could use some optimization.</p>
+                        <p>  Currently, this step is implemented like this:</p>
+                        <pre><code class="language-js">// 4. For each Record { [[Key]], [[Value]] } e that is an element of entries, do
+for (var e of allowContentIter(entries)) {
   var eKey = e[0];
   var eValue = e[1];
-  //...
+  // ...
 }
 </code></pre>
-                        <p>  The worst case for this is that is loops through the entire <code>entries</code>. The result is a runtime of <strong><code>O(n)</code></strong> where <code>n</code> is the size of the <code>Map</code>.
-                            This is rather slow, considering a lookup in maps should be <strong><code>~O(1)</code></strong>, given an efficient <code>HashTable</code> implementation.
-                            Therefore, we decided to try optimizing this line.</p>
-                        <p>  <strong>Demonstration: Create a new file; Runtime.js with the code below and run the script with <code>./mach build</code> and <code>./mach run Runtime.js</code></strong></p>
-                        <details>
-                            <summary>Runtime script</summary>
-
-                            <pre><code class="language-js">  const iterations = 1000;
+                        <p>  In the worst case scenario, the loop would go through all of the <code>entries</code>, resulting in linear time complexity (that is, <code>O(n)</code> where <code>n</code> is the size of the <code>Map</code>).
+                            This is rather slow, especially considering that a lookup in maps could be done in a constant time (<code>~O(1)</code>), given an efficient <code>HashTable</code> implementation.
+                            In this section, we use this fact to optimize the implementation of the step 4 in the specification.</p>
+                        <p>  Before proceeding, we informally demonstrate the performance of the current design of <code>upsert</code>.
+                            In the code below, we measure the runtime of of updating or inserting key-value pairs into a <code>Map</code> object:
+                            using the <code>upsert</code> method vs. using <code>has</code>, <code>get</code>, and <code>set</code>.
+                            The <code>measureRuntime</code> function is used to execute and log the execution time of each approach over a fixed number of iterations.</p>
+                        <p>  We can create a new file <code>Runtime.js</code> with the code below and run it with: <code>./mach build</code> and <code>./mach run Runtime.js</code>.</p>
+                        <pre><code class="language-js">const iterations = 1000;
 
 // Function to measure runtime of a given block of code
 function measureRuntime(callback, description) {
@@ -1353,7 +1356,7 @@ <h2>Optimization</h2>
     console.log(`Runtime: ${runtime} milliseconds \n`);
 }
 
-// test upsert for e record of entries
+// test `upsert` for e record of entries
 function withUpsert() {
     const m = new Map();
 
@@ -1364,7 +1367,7 @@ <h2>Optimization</h2>
     }
 }
 
-//test without upsert
+// test without `upsert`
 function withoutUpsert() {
     const m = new Map();
 
@@ -1379,109 +1382,118 @@ <h2>Optimization</h2>
     }
 }
 
-console.log(&quot;Starting tests...&quot;);
-measureRuntime(withUpsert, &quot;Test upsert for &quot; + iterations + &quot; iterations&quot;);
-measureRuntime(withoutUpsert, &quot;Test without upsert for &quot; + iterations + &quot; iterations&quot;);
-</code></pre>
-                        </details>
-
-
-                        <p>  One solution we had, was to check if the entry was in the <code>Map</code>, by using <code>Map::has</code>.
-                            The problem with this, is that this method is not currently exposed to self-hosted JavaScript™ code. The reason for this
-                            is seemingly because there has not been any need for the <code>Map::has</code> method in self-hosted code previously.</p>
-                        <p>  <strong>Exposing <code>std_Map_has</code> to self-hosted code</strong></p>
-                        <p>  <code>Selfhosting.cpp</code></p>
-                        <pre><code class="language-cpp">
+console.log(&quot;Starting tests ...&quot;);
+measureRuntime(withUpsert, &quot;Test `upsert` for &quot; + iterations + &quot; iterations&quot;);
+measureRuntime(withoutUpsert, &quot;Test without `upsert` for &quot; + iterations + &quot; iterations&quot;);
+</code></pre>
+                        <p>  Now we are ready to consider how the step 4 in the specification can be implemented in a more optimal way.
+                            One solution we could have is to check if an entry was in the <code>Map</code> using the function <a href="https://262.ecma-international.org/#sec-map.prototype.has"><code>Map::has</code></a>.
+                            The problem with this solution, however, is that this method is not currently exposed to self-hosted JavaScript™ code.
+                            An apparent reason for this is that there has not been any need for the <code>Map::has</code> method in self-hosted code previously.</p>
+                        <p>  Recall the <code>Map</code> methods exposed to self-hosted code in <a href="https://searchfox.org/mozilla-central/source/js/src/vm/SelfHosting.cpp#2383"><code>SelfHosting.cpp</code></a>:</p>
+                        <pre><code class="language-cpp">  // This code is from: /js/src/vm/SelfHosting.cpp
+  // ...
   // Standard builtins used by self-hosting.
-  //...
+  // ...
   JS_FN(&quot;std_Map_entries&quot;, MapObject::entries, 0, 0),
   JS_FN(&quot;std_Map_get&quot;, MapObject::get, 1, 0),
   JS_FN(&quot;std_Map_set&quot;, MapObject::set, 2, 0),
-  //...
+  // ...
 </code></pre>
-                        <p>  We want to add this line so that we can use <code>has</code> in our optimized implementation.</p>
-                        <pre><code class="language-cpp">
-  JS_INLINABLE_FN(&quot;std_Map_has&quot;, MapObject::has, 1, 0, MapHas),
+                        <p>  We can expose <code>std_Map_has</code> to self-hosted code by adding the following line in <code>SelfHosting.cpp</code>:</p>
+                        <pre><code class="language-cpp">  JS_INLINABLE_FN(&quot;std_Map_has&quot;, MapObject::has, 1, 0, MapHas),
 </code></pre>
-                        <p>  Copy the line and paste it into <code>js/src/vm/Selfhosting.cpp</code>, before <code>MapObject::set</code> (to ensure consistency across files).</p>
-                        <details>
-                            <summary>Solution</summary>
-
-                            <pre><code class="language-cpp">
-// Standard builtins used by self-hosting.
-//...
-JS_FN(&quot;std_Map_entries&quot;, MapObject::entries, 0, 0),
-JS_FN(&quot;std_Map_get&quot;, MapObject::get, 1, 0),
-JS_INLINABLE_FN(&quot;std_Map_has&quot;, MapObject::has, 1, 0, MapHas),
-JS_FN(&quot;std_Map_set&quot;, MapObject::set, 2, 0),
-//...
+                        <p>To ensure consistency across files, we add this line before <code>JS_FN(&quot;std_Map_set&quot;, MapObject::set, 2, 0),</code>, resulting in the following:</p>
+                        <pre><code class="language-cpp">    // This code is from: /js/src/vm/SelfHosting.cpp
+    // ...
+    // Standard builtins used by self-hosting.
+    // ...
+    JS_FN(&quot;std_Map_entries&quot;, MapObject::entries, 0, 0),
+    JS_FN(&quot;std_Map_get&quot;, MapObject::get, 1, 0),
+    JS_INLINABLE_FN(&quot;std_Map_has&quot;, MapObject::has, 1, 0, MapHas), // we have added this line
+    JS_FN(&quot;std_Map_set&quot;, MapObject::set, 2, 0),
+    // ...
 </code></pre>
-                        </details>
-
-                        <p>  We also need to make the <code>has</code> method publicly exposed in <code>MapObject.h</code> to use it in self-hosted code.</p>
-                        <p>  In <code>MapObject.h</code>, move this line from <strong>private</strong> to <strong>public</strong>.</p>
+                        <p>  We also need to make the method <code>has</code> publicly exposed in <a href="https://searchfox.org/mozilla-central/source/js/src/builtin/MapObject.h"><code>MapObject.h</code></a>.
+                            To do this, we replace the visibility modifier of <a href="https://searchfox.org/mozilla-central/source/js/src/builtin/MapObject.h#217">the method with signature</a>  </p>
                         <pre><code class="language-cpp">[[nodiscard]] static bool has(JSContext* cx, unsigned argc, Value* vp);
 </code></pre>
-                        <details>
-                            <summary>(This could be tricky) Let's break down the structure of the file: </summary>
-
-                            <pre><code class="language-cpp">  class MapObject : public NativeObject {
+                        <p>  from <a href="https://searchfox.org/mozilla-central/source/js/src/builtin/MapObject.h#186"><code>private</code></a> to <a href="https://searchfox.org/mozilla-central/source/js/src/builtin/MapObject.h#109"><code>public</code></a>.</p>
+                        <pre><code class="language-cpp">  // This code is from: /js/src/builtin/MapObject.h
+  // ...
+  class MapObject : public NativeObject {
     public:
-      //...
+      // ...
       const ValueMap* getData() { return getTableUnchecked(); }
 
       [[nodiscard]] static bool get(JSContext* cx, unsigned argc, Value* vp);
       [[nodiscard]] static bool set(JSContext* cx, unsigned argc, Value* vp);
-      //add has here
+      // add `has` here
 
-      //...
+      // ...
 
     private:
-      //...
+      // ...
       [[nodiscard]] static bool size_impl(JSContext* cx, const CallArgs&amp; args);
       [[nodiscard]] static bool size(JSContext* cx, unsigned argc, Value* vp);
       [[nodiscard]] static bool get_impl(JSContext* cx, const CallArgs&amp; args);
       [[nodiscard]] static bool has_impl(JSContext* cx, const CallArgs&amp; args);
-      [[nodiscard]] static bool has(JSContext* cx, unsigned argc, Value* vp); //remove this line
+      [[nodiscard]] static bool has(JSContext* cx, unsigned argc, Value* vp); // remove this line
       [[nodiscard]] static bool set_impl(JSContext* cx, const CallArgs&amp; args);
       [[nodiscard]] static bool delete_impl(JSContext* cx, const CallArgs&amp; args);
       [[nodiscard]] static bool delete_(JSContext* cx, unsigned argc, Value* vp);
       [[nodiscard]] static bool keys_impl(JSContext* cx, const CallArgs&amp; args);
-      //...
+      // ...
   }
 </code></pre>
-                        </details>
+                        <p>This will enable us to use <a href="https://262.ecma-international.org/#sec-map.prototype.has"><code>has</code></a> in our optimized implementation: in self-hosted JavaScript™, we will be able to call this method using <code>callFunction</code> and passing <code>std_Map_has</code> as an argument.</p>
+                        <h3 id="optimizing-the-implementation-of-upsert">Optimizing the implementation of <code>upsert</code></h3>
+                        <p>  We can now modify our implementation of the <code>upsert</code> method to use <code>std_Map_has</code> instead of a <em><code>for ... of</code></em> loop and <a href="https://262.ecma-international.org/#sec-samevaluezero"><code>SameValueZero</code></a>.</p>
+                        <pre><code class="language-js">function MapUpsert(key, value) {
+  // 1. Let M be the this value.
+  var M = this;
 
-                        <p>  The <code>std_Map_has</code> method should now be available in self-hosted JavaScript™.</p>
-                        <h3 id="optimize-the-function">Optimize the function</h3>
-                        <p>  With <code>has</code> now exposed to self-hosted code, alter your implementation to use <code>std_Map_has</code> instead of a <code>for-of</code> loop
-                            and <code>SameValueZero</code>.</p>
-                        <details>
-                            <summary>Solution</summary>
+  // 2. Perform ? RequireInternalSlot(M, [[MapData]]).
+  if (!IsObject(M) || (M = GuardToMapObject(M)) === null) {
+    return callFunction(
+      CallMapMethodIfWrapped,
+      this,
+      key,
+      value,
+      &quot;MapUpsert&quot;
+    );
+  }
 
-                            <pre><code class="language-js">
-  function MapUpsert(key, value) {
-    var M = this;
-
-    if (!IsObject(M) || (M = GuardToMapObject(M)) === null) {
-        return callFunction(
-            CallMapMethodIfWrapped,
-            this,
-            key,
-            value,
-            &quot;MapUpsert&quot;
-        );
-    }
+  // 3. Let entries be the List that is M.[[MapData]].
+  // 4. For each Record { [[Key]], [[Value]] } e that is an element of entries, do
+  // 4a. If e.[[Key]] is not empty and SameValueZero(e.[[Key]], key) is true, return e.[[Value]].
+  if (callFunction(std_Map_has, M, key)) {
+    return callFunction(std_Map_get, M, key);
+  }
 
-    if (callFunction(std_Map_has, M, key)) {
-      return callFunction(std_Map_get, M, key);
-    }
+  // 5. Set e.[[Value]] to value.
+  callFunction(std_Map_set, M, key, value);
 
-    callFunction(std_Map_set, M, key, value);
-      return value;
-  }
+  // 6. Return e.[[Value]].
+  return value;
+}
 </code></pre>
-                        </details>
+                        <p>It is important to note that now our implementation does not follow the exact structure of the <code>upsert</code> specification text.
+                            This is a common practice - as long as we make sure that the produced
+                            <a href="https://github.com/tc39/how-we-work/blame/cc47a79340a773876cb03371dc2d46b9d9ce9695/how-to-read.md#L7"><strong>observable behavior</strong></a>
+                            is the same as the one defined by the spec.
+                            This ensures compatibility and correctness while allowing flexibility in optimization or internal design.</p>
+                        <p>We quote here a <a href="https://github.com/tc39/how-we-work/blame/cc47a79340a773876cb03371dc2d46b9d9ce9695/how-to-read.md#L7">comment</a> on the <a href="https://github.com/tc39/how-we-work"><em>How We Work</em></a> document by TC39:</p>
+                        <blockquote>
+                            <p><em>Specification text is meant to be interpreted abstractly. Only the observable semantics, that is, the behavior when JavaScript™ code is executed, need to match the specification. Implementations can use any strategy they want to make that happen, including using different algorithms that reach the same result.</em></p>
+                        </blockquote>
+                        <p>Further discussion on this can be found <a href="https://github.com/tc39/how-we-work/issues/104">here</a>.</p>
+                        <p>Note also that we don&#39;t need to change the specification text to reflect our optimized implementation:
+                            the specification is intended to define behavior at an abstract level, providing a precise but implementation-agnostic guide for JavaScript™ engines.
+                            This abstraction ensures that the specification remains broadly applicable,
+                            leaving room for diverse implementations while guaranteeing consistent observable behavior across compliant engines.
+                        </p>
+
                     </div>
                 </section>
 
