Expanded test and added comment for clarification

algoidurovic · Oct 15, 2021 · bb1eb20 · bb1eb20
1 parent e0795fc
commit bb1eb20
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 29 deletions.
diff --git a/pyteal/compiler/constants.py b/pyteal/compiler/constants.py
@@ -132,6 +132,8 @@ def createConstantBlocks(ops: List[TealComponent]) -> List[TealComponent]:
     sortedInts = sorted(intFreqs, key=lambda x: intFreqs[x], reverse=True)
     sortedBytes = sorted(byteFreqs, key=lambda x: byteFreqs[x], reverse=True)
 
+    # Use Op.pushint if the constant does not occur in the top 4 most frequent and is smaller than
+    # 2 ** 7 to improve performance and save block space.
     intBlock = [
         val
         for i, val in enumerate(sortedInts)

diff --git a/pyteal/compiler/constants_test.py b/pyteal/compiler/constants_test.py
@@ -589,32 +589,33 @@ def test_createConstantBlocks_small_constant():
     and it can be stored in one varuint it byte then Op.pushint is used.
     """
 
-    ops = [
-        TealOp(None, Op.int, 0),
-        TealOp(None, Op.int, 0),
-        TealOp(None, Op.int, 1),
-        TealOp(None, Op.int, 1),
-        TealOp(None, Op.int, 2),
-        TealOp(None, Op.int, 2),
-        TealOp(None, Op.int, 3),
-        TealOp(None, Op.int, 3),
-        TealOp(None, Op.int, 4),
-        TealOp(None, Op.int, 4),
-    ]
-
-    expected = [
-        TealOp(None, Op.intcblock, 0, 1, 2, 3),
-        TealOp(None, Op.intc_0, "//", 0),
-        TealOp(None, Op.intc_0, "//", 0),
-        TealOp(None, Op.intc_1, "//", 1),
-        TealOp(None, Op.intc_1, "//", 1),
-        TealOp(None, Op.intc_2, "//", 2),
-        TealOp(None, Op.intc_2, "//", 2),
-        TealOp(None, Op.intc_3, "//", 3),
-        TealOp(None, Op.intc_3, "//", 3),
-        TealOp(None, Op.pushint, 4, "//", 4),
-        TealOp(None, Op.pushint, 4, "//", 4),
-    ]
-
-    actual = createConstantBlocks(ops)
-    assert actual == expected
+    for cur in range(4, 2 ** 7):
+        ops = [
+            TealOp(None, Op.int, 0),
+            TealOp(None, Op.int, 0),
+            TealOp(None, Op.int, 1),
+            TealOp(None, Op.int, 1),
+            TealOp(None, Op.int, 2),
+            TealOp(None, Op.int, 2),
+            TealOp(None, Op.int, 3),
+            TealOp(None, Op.int, 3),
+            TealOp(None, Op.int, cur),
+            TealOp(None, Op.int, cur),
+        ]
+
+        expected = [
+            TealOp(None, Op.intcblock, 0, 1, 2, 3),
+            TealOp(None, Op.intc_0, "//", 0),
+            TealOp(None, Op.intc_0, "//", 0),
+            TealOp(None, Op.intc_1, "//", 1),
+            TealOp(None, Op.intc_1, "//", 1),
+            TealOp(None, Op.intc_2, "//", 2),
+            TealOp(None, Op.intc_2, "//", 2),
+            TealOp(None, Op.intc_3, "//", 3),
+            TealOp(None, Op.intc_3, "//", 3),
+            TealOp(None, Op.pushint, cur, "//", cur),
+            TealOp(None, Op.pushint, cur, "//", cur),
+        ]
+
+        actual = createConstantBlocks(ops)
+        assert actual == expected