From b4d5848597cb2a2e8535725d2c2a18183d4696cc Mon Sep 17 00:00:00 2001
From: Elizabeth Mattijsen <liz@raku.rocks>
Date: Tue, 12 Apr 2022 11:37:57 +0200
Subject: [PATCH] Speed up "sorted_keys" sub

- faster collection of keys using nqp::iterator/nqp::iterkey_s
- one check for > 2
- one check for == 2 and order
- decrement in while condition, account for that in initialization
- document it's actually sorting in *reverse* order

Timings before after (in msecs, 1M iterations):
elems      0       2       10
before   363     579     3717
after    332     508     3274

MasterDuke++ for nudging
---
 src/core/Hash.nqp       | 44 ++++++++++++++++++++---------------------
 t/nqp/018-associative.t | 21 +++++++++++++++++++-
 2 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/src/core/Hash.nqp b/src/core/Hash.nqp
index 084e6e68a7..962d21a5a0 100644
--- a/src/core/Hash.nqp
+++ b/src/core/Hash.nqp
@@ -6,12 +6,14 @@ sub hash(*%new) {
 # so a bubble sort would be fine. However, the
 # number can get much larger (e.g., when profiling
 # a build of the Rakudo settings), so use a heapsort
-# instead.
+# instead.  Note that this sorts in **reverse** order.
 sub sorted_keys($hash) {
     my @keys := nqp::list_s();
-    for $hash {
-        nqp::push_s(@keys, $_.key);
-    }
+    my $iter := nqp::iterator($hash);
+    nqp::while(
+      $iter,
+      nqp::push_s(@keys,nqp::iterkey_s(nqp::shift($iter)))
+    );
 
     sub sift_down(@a, int $start, int $end) {
         my int $root := $start;
@@ -40,26 +42,24 @@ sub sorted_keys($hash) {
     }
 
     my int $count := +@keys;
-    if $count < 3 {
-        if $count == 2 && nqp::atpos_s(@keys, 0) gt nqp::atpos_s(@keys, 1) {
-            nqp::push_s(@keys, nqp::shift_s(@keys));
+    if $count > 2 {
+        my int $start := $count / 2;
+        my int $end := $count - 1;
+        while --$start >= 0 {
+            sift_down(@keys, $start, $end);
         }
-        return @keys;
-    }
-    my int $start := $count / 2 - 1;
-    my int $end := $count - 1;
-    while $start >= 0 {
-        sift_down(@keys, $start, $end);
-        $start := $start - 1;
-    }
 
-    while $end > 0 {
-        my str $swap := nqp::atpos_s(@keys, $end);
-        nqp::bindpos_s(@keys, $end, nqp::atpos_s(@keys, 0));
-        nqp::bindpos_s(@keys, 0, $swap);
-        $end := $end - 1;
-        sift_down(@keys, 0, $end);
+        while $end > 0 {
+            my str $swap := nqp::atpos_s(@keys, $end);
+            nqp::bindpos_s(@keys, $end, nqp::atpos_s(@keys, 0));
+            nqp::bindpos_s(@keys, 0, $swap);
+            $end := $end - 1;
+            sift_down(@keys, 0, $end);
+        }
+    }
+    elsif $count == 2 && nqp::atpos_s(@keys, 0) lt nqp::atpos_s(@keys, 1) {
+        nqp::push_s(@keys, nqp::shift_s(@keys));
     }
 
-    return @keys;
+    @keys
 }
diff --git a/t/nqp/018-associative.t b/t/nqp/018-associative.t
index 8e9022d81d..da41ba0200 100644
--- a/t/nqp/018-associative.t
+++ b/t/nqp/018-associative.t
@@ -1,6 +1,6 @@
 # check hash access methods
 
-plan(19);
+plan(23);
 
 my %h;
 
@@ -48,3 +48,22 @@ sub as_return_value() {
 }
 
 ok(nqp::eqaddr(as_return_value(), NQPMu), 'getting a NQPMu for a missing hash member when used a s return value');
+
+my @keys := <a b c d e f g h i j>;
+my @values := 1,2,3,4,5,6,7,8,9,10;
+
+my %sh;
+for @values {
+    %sh{@keys[$_-1]} := $_;
+}
+
+is(nqp::join("",sorted_keys(%sh)), nqp::flip(nqp::join("",@keys)), 'did we get sorted keys');
+
+my %eh;
+is(nqp::join("",sorted_keys(%eh)), "", 'did we get no keys');
+
+%eh<a> := 1;
+is(nqp::join("",sorted_keys(%eh)), "a", 'did we get one key');
+
+%eh<b> := 1;
+is(nqp::join("",sorted_keys(%eh)), "ba", 'did we get two keys');