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

[Compiler] Sync with master #3744

Merged
merged 203 commits into from
Jan 29, 2025
Merged

[Compiler] Sync with master #3744

merged 203 commits into from
Jan 29, 2025

Conversation

turbolent
Copy link
Member

@turbolent turbolent commented Jan 29, 2025

Work towards #3742

Description

Merge master into the feature branch.

Conflict resolution:
commit 359ce2ab0abbcf73763dd0ec1edcdf3e4dea66df
Merge: 4703a34a2 68a046054
Author: Bastian Müller <[email protected]>
Date:   Wed Jan 29 10:54:04 2025 -0800

    Merge branch 'master' into bastian/sync-compiler

diff --git a/interpreter/encode.go b/interpreter/encode.go
remerge CONFLICT (content): Merge conflict in interpreter/encode.go
index 101a0d46d..aad1bfa82 100644
--- a/interpreter/encode.go
+++ b/interpreter/encode.go
@@ -1684,15 +1684,7 @@ func (c CompositeTypeInfo) IsComposite() bool {
 	return true
 }
 
-<<<<<<< 4703a34a2 (Merge pull request #3735 from onflow/supun/compiler-lint)
-func (c CompositeTypeInfo) Identifier() string {
-	return string(c.Location.TypeID(nil, c.QualifiedIdentifier))
-}
-
 func (c CompositeTypeInfo) Copy() atree.TypeInfo {
-=======
-func (c compositeTypeInfo) Copy() atree.TypeInfo {
->>>>>>> 68a046054 (Merge pull request #3743 from onflow/release/v1.3.1)
 	// Return c as is because c is a value type.
 	return c
 }
diff --git a/interpreter/value_composite.go b/interpreter/value_composite.go
remerge CONFLICT (content): Merge conflict in interpreter/value_composite.go
index caf1c3052..db1ce4c49 100644
--- a/interpreter/value_composite.go
+++ b/interpreter/value_composite.go
@@ -1953,11 +1953,10 @@ func (v *CompositeValue) ForEach(
 	}
 }
 
-<<<<<<< 4703a34a2 (Merge pull request #3735 from onflow/supun/compiler-lint)
 func (v *CompositeValue) AtreeMap() *atree.OrderedMap {
 	return v.dictionary
-=======
+}
+
 func (v *CompositeValue) Inlined() bool {
 	return v.dictionary.Inlined()
->>>>>>> 68a046054 (Merge pull request #3743 from onflow/release/v1.3.1)
 }
diff --git a/interpreter/value_dictionary.go b/interpreter/value_dictionary.go
remerge CONFLICT (content): Merge conflict in interpreter/value_dictionary.go
index 3946fdc80..8ee1be485 100644
--- a/interpreter/value_dictionary.go
+++ b/interpreter/value_dictionary.go
@@ -1599,15 +1599,14 @@ func (v *DictionaryValue) SetType(staticType *DictionaryStaticType) {
 	}
 }
 
-<<<<<<< 4703a34a2 (Merge pull request #3735 from onflow/supun/compiler-lint)
 func (v *DictionaryValue) AtreeMap() *atree.OrderedMap {
 	return v.dictionary
 }
 
 func (v *DictionaryValue) ElementSize() uint {
 	return v.elementSize
-=======
+}
+
 func (v *DictionaryValue) Inlined() bool {
 	return v.dictionary.Inlined()
->>>>>>> 68a046054 (Merge pull request #3743 from onflow/release/v1.3.1)
 }
diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go
remerge CONFLICT (content): Merge conflict in runtime/runtime_test.go
index ee3d7cb77..4394b73e9 100644
--- a/runtime/runtime_test.go
+++ b/runtime/runtime_test.go
@@ -11912,7 +11912,6 @@ func TestRuntimeBuiltInFunctionConfusion(t *testing.T) {
 	require.ErrorAs(t, err, &redeclarationError)
 }
 
-<<<<<<< 4703a34a2 (Merge pull request #3735 from onflow/supun/compiler-lint)
 func BenchmarkContractFunctionInvocation(b *testing.B) {
 
 	runtime := NewTestInterpreterRuntime()
@@ -11938,37 +11937,10 @@ func BenchmarkContractFunctionInvocation(b *testing.B) {
                   // return self.id
                   return Test.helloText()
               }
-=======
-func TestRuntimeInvocationReturnTypeInferenceFailure(t *testing.T) {
-
-	t.Parallel()
-
-	address := common.MustBytesToAddress([]byte{0x1})
-
-	newRuntimeInterface := func() Interface {
-
-		return &TestRuntimeInterface{
-			Storage: NewTestLedger(nil, nil),
-			OnGetSigningAccounts: func() ([]common.Address, error) {
-				return []common.Address{address}, nil
-			},
-		}
-	}
-
-	runtime := NewTestInterpreterRuntime()
-
-	nextTransactionLocation := NewTransactionLocationGenerator()
-
-	tx := []byte(`
-      transaction{
-          prepare(signer: auth(Storage) &Account){
-              let functions = [signer.storage.save].reverse()
->>>>>>> 68a046054 (Merge pull request #3743 from onflow/release/v1.3.1)
           }
       }
     `)
 
-<<<<<<< 4703a34a2 (Merge pull request #3735 from onflow/supun/compiler-lint)
 	deploy := DeploymentTransaction("Test", contract)
 
 	var accountCode []byte
@@ -12045,7 +12017,36 @@ func TestRuntimeInvocationReturnTypeInferenceFailure(t *testing.T) {
 		)
 		require.NoError(b, err)
 	}
-=======
+}
+
+func TestRuntimeInvocationReturnTypeInferenceFailure(t *testing.T) {
+
+	t.Parallel()
+
+	address := common.MustBytesToAddress([]byte{0x1})
+
+	newRuntimeInterface := func() Interface {
+
+		return &TestRuntimeInterface{
+			Storage: NewTestLedger(nil, nil),
+			OnGetSigningAccounts: func() ([]common.Address, error) {
+				return []common.Address{address}, nil
+			},
+		}
+	}
+
+	runtime := NewTestInterpreterRuntime()
+
+	nextTransactionLocation := NewTransactionLocationGenerator()
+
+	tx := []byte(`
+      transaction{
+          prepare(signer: auth(Storage) &Account){
+              let functions = [signer.storage.save].reverse()
+          }
+      }
+    `)
+
 	err := runtime.ExecuteTransaction(
 		Script{
 			Source: tx,
@@ -12383,5 +12384,4 @@ func TestRuntimeSomeValueChildContainerMutation(t *testing.T) {
 		logs = runTransaction(buyTicketTx)
 		assert.Equal(t, []string{"5.00000000", "10.00000000"}, logs)
 	})
->>>>>>> 68a046054 (Merge pull request #3743 from onflow/release/v1.3.1)
 }

  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

turbolent and others added 30 commits October 30, 2024 15:08
This commit:
1. Combines all domain (non-atree) payloads into
   one account (non-atree) payload per account.
2. Combines all domain (atree) storage maps into
   one account (atree) storage map per account.
3. Uses on-the-fly (OTF) migration when an
   account is modified (write ops).

Currently, accounts store data on-chain under pre-defined domains,
such as "storage".  Each domain requires domain payload
(8-byte non-atree payload) and domain storage map payload (atree payload).
Also, each payload requires ~2 mtrie nodes (~2x96 byte overhead).

New domains were added in Cadence 1.0 and domain payloads count
increased to 150 million on Sept. 4 (was 80 million pre-spork).
Nearly 25% of total payloads on-chain are 8-byte domain payloads.
Each account on mainnet has an average of ~4 domain payloads and
~4 domain storage maps.

This commit creates 1 account (non-atree) payload and 1 account
(atree) storage map per account, eliminating all domain (non-atree)
payloads and domain storage maps for that given account.

Based on preliminary estimates using Sept. 17, 2024 mainnet state,
this approach can:
- eliminate mtrie nodes: -425 million (-28.5%)
- reduce payload count: -174 million (also -28.5%)

This commit also includes on-the-fly migration so we can see
improvements to accounts that have write activity without requiring
downtime.  Given this, we won't see the full benefits/impact until
all accounts (including idle accounts) are eventually migrated
(e.g. using full migration or other means).
Currently, various storage domains are defined in
different packages, such as:
- common
- runtime
- stdlib

This commit moves storage domains from different
packages to a single place: common/storagedomain.go.

This change will help us avoid import cycles when
using domains across different packages.
Currently, we use domain string to get StorageMap.  Even though
we use pre-defined domain strings, this can still be error-prone.

This commit modifies GetStorageMap() to use common.StorageDomain
instead of string.
…ce-domain-regsiters

Combine domain payloads and provide on-the-fly migration
atree.TypeInfo interface used to include Identifier() function.
However, Identifier() was removed later and is no longer part of
atree.TypeInfo interface.

This commit removes Identifier() function from structs
implementing atree.TypeInfo interface.
…s-for-atree-typeinfo

Remove unused Identifier() function from structs implementing atree.TypeInfo
Refactor storage domains to prevent import cycles and simplify maintenance
This commit replaces domain string, such as "storage",
"public", "private", etc. with domain enum (integer)
as the key for AccountStorageMap.

Also, this uses CBOR to store the domain integer in shortest form.

This optimization further improves storage efficiency
of domain payloads of PR 3664.
…mbine-domain-payloads

Sync `feature/combine-domain-payloads` branch with master
@turbolent turbolent requested a review from jsproz January 29, 2025 18:57
@turbolent turbolent requested a review from SupunS as a code owner January 29, 2025 18:57
Copy link

Cadence Benchstat comparison

This branch with compared with the base branch onflow:feature/compiler commit 4703a34
The command for i in {1..N}; do go test ./... -run=XXX -bench=. -benchmem -shuffle=on; done was used.
Bench tests were run a total of 7 times on each branch.

Collapsed results for better readability

Copy link
Member

@SupunS SupunS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for updating!

@SupunS SupunS merged commit 3a904fd into feature/compiler Jan 29, 2025
9 checks passed
@turbolent turbolent deleted the bastian/sync-compiler branch January 29, 2025 21:58
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

Successfully merging this pull request may close these issues.

7 participants