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-v2] add test cases for lambda #14791

Merged
merged 4 commits into from
Oct 21, 2024

Conversation

brmataptos
Copy link
Contributor

@brmataptos brmataptos commented Sep 28, 2024

Description

Add a /lambda/ test subdir for move-compiler-v2, and test it with LAMBDA experiments off (.exp) and on (.lambda.exp).

How Has This Been Tested?

This is just a test.

Key Areas to Review

Observe baselines. Can you think of any other useful tests to include? If not, just approve, please.

Type of Change

  • New feature
  • Bug fix
  • Breaking change
  • Performance improvement
  • Refactoring
  • Dependency update
  • Documentation update
  • Tests

Which Components or Systems Does This Change Impact?

  • Validator Node
  • Full Node (API, Indexer, etc.)
  • Move/Aptos Virtual Machine
  • Aptos Framework
  • Aptos CLI/SDK
  • Developer Infrastructure
  • Move Compiler
  • Other (specify)

Checklist

  • I have read and followed the CONTRIBUTING doc
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I identified and added all stakeholders and component owners affected by this change as reviewers
  • I tested both happy and unhappy path of the functionality
  • I have made corresponding changes to the documentation

Copy link

trunk-io bot commented Sep 28, 2024

⏱️ 3h 43m total CI duration on this PR
Slowest 15 Jobs Cumulative Duration Recent Runs
rust-cargo-deny 15m 🟩🟩🟩🟩🟩 (+4 more)
rust-move-unit-coverage 15m 🟩
rust-move-unit-coverage 12m 🟩
rust-move-unit-coverage 11m 🟩
rust-move-unit-coverage 10m 🟩
rust-move-tests 10m 🟩
rust-move-unit-coverage 10m 🟩
rust-move-tests 10m 🟩
rust-move-unit-coverage 10m 🟩
rust-move-unit-coverage 10m 🟩
general-lints 10m 🟩🟩🟩🟩🟩 (+4 more)
rust-move-tests 10m 🟩
rust-move-tests 10m 🟩
rust-move-tests 10m 🟩
rust-move-tests 10m 🟩

settingsfeedbackdocs ⋅ learn more about trunk.io

@brmataptos brmataptos marked this pull request as ready for review September 28, 2024 04:24
@brmataptos brmataptos force-pushed the 09-24-fix_14633_fix_error_messages_for_lambdas_identify_change_points_for_more_complete_lambda_implementation branch from 61459eb to 0f3224a Compare September 28, 2024 04:32
@brmataptos brmataptos force-pushed the 09-27-extend_parser_for_lambda_types branch from 78767d3 to 7f59256 Compare September 28, 2024 04:32
Copy link

codecov bot commented Sep 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.1%. Comparing base (918b643) to head (c99de22).

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #14791   +/-   ##
=======================================
  Coverage    60.1%    60.1%           
=======================================
  Files         856      856           
  Lines      210845   210845           
=======================================
+ Hits       126742   126761   +19     
+ Misses      84103    84084   -19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

options: opts
.clone()
// .set_experiment(Experiment::AST_SIMPLIFY, true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove this commented out code?

@@ -213,7 +214,7 @@ const TEST_CONFIGS: Lazy<BTreeMap<&str, TestConfig>> = Lazy::new(|| {
TestConfig {
Copy link
Contributor

Choose a reason for hiding this comment

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

In the comment above, add "lambdas" as well.



Diagnostics:
error: local `a_less_b` of type `|(T, T)|bool` does not have the `copy` ability
Copy link
Contributor

Choose a reason for hiding this comment

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

I am assuming we should we ignore weird errors in lambda.exp for now, because these will change once lambdas are implemented?

@brmataptos brmataptos force-pushed the 09-27-extend_parser_for_lambda_types branch 2 times, most recently from ae73f37 to 243ea0f Compare September 30, 2024 18:43
@brmataptos brmataptos force-pushed the 09-24-fix_14633_fix_error_messages_for_lambdas_identify_change_points_for_more_complete_lambda_implementation branch from a8dba3c to 9b0ccd6 Compare October 3, 2024 01:42
@brmataptos brmataptos force-pushed the 09-27-extend_parser_for_lambda_types branch from 243ea0f to f3d30eb Compare October 3, 2024 01:42
@brmataptos brmataptos force-pushed the 09-24-fix_14633_fix_error_messages_for_lambdas_identify_change_points_for_more_complete_lambda_implementation branch from 9b0ccd6 to e8a6216 Compare October 3, 2024 01:52
@brmataptos brmataptos force-pushed the 09-27-extend_parser_for_lambda_types branch 2 times, most recently from 0cc6890 to 1b54f30 Compare October 3, 2024 04:42
@brmataptos brmataptos force-pushed the 09-24-fix_14633_fix_error_messages_for_lambdas_identify_change_points_for_more_complete_lambda_implementation branch from a050828 to 8a317b8 Compare October 3, 2024 17:01
@brmataptos brmataptos force-pushed the 09-27-extend_parser_for_lambda_types branch from ad600f0 to 017f471 Compare October 3, 2024 17:01
@wrwg wrwg changed the title add test cases for lambda [compiler-vc] add test cases for lambda Oct 3, 2024
@wrwg wrwg changed the title [compiler-vc] add test cases for lambda [compiler-v2] add test cases for lambda Oct 3, 2024
@brmataptos brmataptos force-pushed the 09-24-fix_14633_fix_error_messages_for_lambdas_identify_change_points_for_more_complete_lambda_implementation branch from 8a317b8 to f72846f Compare October 10, 2024 04:17
@brmataptos brmataptos force-pushed the 09-27-extend_parser_for_lambda_types branch from 017f471 to b7d429a Compare October 10, 2024 04:18
Base automatically changed from 09-24-fix_14633_fix_error_messages_for_lambdas_identify_change_points_for_more_complete_lambda_implementation to main October 10, 2024 04:56
@brmataptos brmataptos force-pushed the 09-27-extend_parser_for_lambda_types branch from b7d429a to c99de22 Compare October 10, 2024 05:25
@brmataptos brmataptos enabled auto-merge (squash) October 10, 2024 05:27

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@brmataptos brmataptos force-pushed the 09-27-extend_parser_for_lambda_types branch 2 times, most recently from d3254ad to 7e21319 Compare October 18, 2024 18:08

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@brmataptos brmataptos force-pushed the 09-27-extend_parser_for_lambda_types branch from 7e21319 to 7cfe2c4 Compare October 21, 2024 07:10

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

✅ Forge suite compat success on b29f09f57e898d8d211c8bc3e303f6e50bba2266 ==> 7cfe2c4832ed13365b51ce987cd2afc460d0880b

Compatibility test results for b29f09f57e898d8d211c8bc3e303f6e50bba2266 ==> 7cfe2c4832ed13365b51ce987cd2afc460d0880b (PR)
1. Check liveness of validators at old version: b29f09f57e898d8d211c8bc3e303f6e50bba2266
compatibility::simple-validator-upgrade::liveness-check : committed: 12875.49 txn/s, latency: 2519.90 ms, (p50: 1900 ms, p70: 2100, p90: 2800 ms, p99: 18100 ms), latency samples: 517480
2. Upgrading first Validator to new version: 7cfe2c4832ed13365b51ce987cd2afc460d0880b
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 5998.40 txn/s, latency: 4796.86 ms, (p50: 5400 ms, p70: 5700, p90: 6000 ms, p99: 6100 ms), latency samples: 113140
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 5413.89 txn/s, latency: 5930.73 ms, (p50: 6400 ms, p70: 6500, p90: 7300 ms, p99: 8000 ms), latency samples: 182800
3. Upgrading rest of first batch to new version: 7cfe2c4832ed13365b51ce987cd2afc460d0880b
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 5513.62 txn/s, latency: 5305.97 ms, (p50: 5800 ms, p70: 6300, p90: 6600 ms, p99: 6700 ms), latency samples: 112060
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 5330.44 txn/s, latency: 6082.05 ms, (p50: 6600 ms, p70: 6900, p90: 7300 ms, p99: 7800 ms), latency samples: 187900
4. upgrading second batch to new version: 7cfe2c4832ed13365b51ce987cd2afc460d0880b
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 8602.64 txn/s, latency: 3177.74 ms, (p50: 3400 ms, p70: 3500, p90: 3900 ms, p99: 4300 ms), latency samples: 156380
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 8043.28 txn/s, latency: 3839.68 ms, (p50: 3500 ms, p70: 3700, p90: 7100 ms, p99: 8100 ms), latency samples: 285960
5. check swarm health
Compatibility test for b29f09f57e898d8d211c8bc3e303f6e50bba2266 ==> 7cfe2c4832ed13365b51ce987cd2afc460d0880b passed
Test Ok

Copy link
Contributor

✅ Forge suite realistic_env_max_load success on 7cfe2c4832ed13365b51ce987cd2afc460d0880b

two traffics test: inner traffic : committed: 13214.35 txn/s, latency: 2998.68 ms, (p50: 2700 ms, p70: 2900, p90: 3300 ms, p99: 9900 ms), latency samples: 5024580
two traffics test : committed: 99.97 txn/s, latency: 1679.18 ms, (p50: 1500 ms, p70: 1600, p90: 1900 ms, p99: 3900 ms), latency samples: 1700
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.231, avg: 0.215", "QsPosToProposal: max: 1.166, avg: 0.945", "ConsensusProposalToOrdered: max: 0.317, avg: 0.307", "ConsensusOrderedToCommit: max: 0.557, avg: 0.507", "ConsensusProposalToCommit: max: 0.862, avg: 0.814"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 1.13s no progress at version 2214098 (avg 0.22s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 7.93s no progress at version 2214096 (avg 7.93s) [limit 15].
Test Ok

Copy link
Contributor

✅ Forge suite framework_upgrade success on b29f09f57e898d8d211c8bc3e303f6e50bba2266 ==> 7cfe2c4832ed13365b51ce987cd2afc460d0880b

Compatibility test results for b29f09f57e898d8d211c8bc3e303f6e50bba2266 ==> 7cfe2c4832ed13365b51ce987cd2afc460d0880b (PR)
Upgrade the nodes to version: 7cfe2c4832ed13365b51ce987cd2afc460d0880b
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1300.36 txn/s, submitted: 1303.54 txn/s, failed submission: 3.18 txn/s, expired: 3.18 txn/s, latency: 2394.44 ms, (p50: 2400 ms, p70: 2700, p90: 3300 ms, p99: 4700 ms), latency samples: 114420
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1267.52 txn/s, submitted: 1270.04 txn/s, failed submission: 2.52 txn/s, expired: 2.52 txn/s, latency: 2427.25 ms, (p50: 2100 ms, p70: 2400, p90: 3600 ms, p99: 6600 ms), latency samples: 110620
5. check swarm health
Compatibility test for b29f09f57e898d8d211c8bc3e303f6e50bba2266 ==> 7cfe2c4832ed13365b51ce987cd2afc460d0880b passed
Upgrade the remaining nodes to version: 7cfe2c4832ed13365b51ce987cd2afc460d0880b
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1146.65 txn/s, submitted: 1149.29 txn/s, failed submission: 2.64 txn/s, expired: 2.64 txn/s, latency: 2498.50 ms, (p50: 2400 ms, p70: 2700, p90: 3600 ms, p99: 5800 ms), latency samples: 104360
Test Ok

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.

3 participants