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

[seqbench] the-good-parts sequence #14909

Closed
wants to merge 3 commits into from

Conversation

cameel
Copy link
Member

@cameel cameel commented Mar 6, 2024

Related to #14406

Do not merge. This is just a test PR to evaluate how the the-good-parts sequence would affect costs in CI.

The sequence is a variant of the previously tested single-pass sequence, but keeping only the parts that seemed to give positive results and with an extra repeated part to keep up with cases where multiple repetitions were still giving the default sequence an edge in bytecode size.

@cameel cameel self-assigned this Mar 6, 2024
@cameel cameel force-pushed the seqbench-sequence-the-good-parts branch from 97e1289 to 60b7222 Compare March 6, 2024 12:31
@cameel
Copy link
Member Author

cameel commented Mar 6, 2024

gas_diff_stats

File name IR optimized Legacy optimized Legacy
array/create_memory_array.sol 9%
array/array_storage_push_empty_length_address.sol 8%
externalContracts/base64.sol 8% -0%
array/array_storage_index_boundary_test.sol 7%
array/delete/bytes_delete_element.sol 7% -0%
array/copying/cleanup_during_multi_element_per_slot_copy.sol 6%
array/copying/bytes_storage_to_storage.sol 5% -0%
array/copying/array_copy_cleanup_uint40.sol 5%
array/copying/storage_memory_packed_dyn.sol 5%
array/byte_array_transitional_2.sol 5%
various/erc20.sol 4% +0%
abiEncoderV2/calldata_array.sol 3%
userDefinedValueType/erc20.sol 3% +0%
array/array_storage_push_pop.sol 3%
byte_array_to_storage_cleanup.sol 3% -0%
inheritance/inherited_function_calldata_memory_interface.sol 3%
array/push/push_no_args_bytes.sol 3%
abiEncoderV2/abi_encode_calldata_slice.sol 3% -0%
abiEncoderV1/abi_encode_calldata_slice.sol 3% -0%
array/array_storage_push_empty.sol 2%
externalContracts/FixedFeeRegistrar.sol 2% +0%
array/array_storage_index_zeroed_test.sol 2%
isoltestTesting/balance_other_contract.sol 2%
externalContracts/prbmath_unsigned.sol 2%
array/dynamic_arrays_in_storage.sol 2%
events/event_indexed_string.sol 1% -0%
abiEncoderV2/abi_encode_v2_in_function_inherited_in_v1_contract.sol 1% -0%
array/pop/array_pop_uint16_transition.sol 1%
array/fixed_arrays_as_return_type.sol 1% -0%
array/push/byte_array_push_transition.sol 1%
array/copying/array_of_struct_memory_to_storage.sol 1%
various/selfdestruct_pre_cancun_multiple_beneficiaries.sol 1%
array/copying/array_of_struct_calldata_to_storage.sol 1%
array/copying/array_copy_clear_storage.sol 1%
array/pop/array_pop_uint24_transition.sol 1%
externalContracts/snark.sol 1%
structs/conversion/recursive_storage_memory.sol 1%
array/copying/memory_dyn_2d_bytes_to_storage.sol 1% -0%
structs/memory_structs_nested_load.sol 1%
array/array_storage_index_access.sol 1%
events/event_dynamic_array_storage_v2.sol 1% +0%
events/event_dynamic_array_storage.sol 1% +0%
externalContracts/prbmath_signed.sol 1%
salted_create/salted_create_with_value.sol 1%
constructor/bytes_in_constructors_packer.sol +0% +0%
various/value_complex.sol 1%
various/value_insane.sol +0%
structs/struct_delete_storage_with_array.sol +0%
array/push/array_push.sol +0%
array/pop/byte_array_pop_long_storage_empty.sol +0%
array/copying/copy_byte_array_to_storage.sol +0% -0%
events/event_dynamic_nested_array_storage_v2.sol +0% +0%
array/copying/storage_memory_nested_from_pointer.sol +0%
array/copying/storage_memory_nested.sol +0%
various/selfdestruct_pre_cancun_redeploy.sol +0%
array/copying/array_copy_storage_storage_different_base.sol +0%
immutable/use_scratch.sol +0%
structs/struct_memory_to_storage_function_ptr.sol +0%
abiEncoderV2/abi_encode_v2.sol +0% +0%
array/copying/array_copy_storage_storage_dynamic_dynamic.sol +0%
array/pop/array_pop_array_transition.sol +0%
array/push/nested_bytes_push.sol +0% -0%
array/constant_var_as_array_length.sol +0% +0%
array/copying/array_of_structs_containing_arrays_calldata_to_storage.sol +0%
array/dynamic_multi_array_cleanup.sol +0%
structs/struct_copy_via_local.sol +0%
array/push/array_push_struct.sol +0%
abiEncoderV1/struct/struct_storage_ptr.sol +0%
libraries/using_library_mappings_public.sol +0%
array/fixed_arrays_in_constructors.sol +0%
array/copying/array_copy_different_packing.sol +0%
constructor/constructor_arguments_external.sol +0%
viaYul/copy_struct_invalid_ir_bug.sol +0%
array/copying/nested_array_element_storage_to_storage.sol +0%
userDefinedValueType/calldata.sol +0% -0%
array/copying/array_copy_storage_to_memory_nested.sol +0% +0%
storage/packed_storage_structs_bytes.sol +0%
array/copying/calldata_array_dynamic_to_storage.sol +0% -0%
array/copying/storage_memory_nested_struct.sol +0%
array/reusing_memory.sol +0%
array/dynamic_array_cleanup.sol +0%
inheritance/value_for_constructor.sol +0%
array/copying/array_nested_calldata_to_storage.sol +0%
constructor/constructor_static_array_argument.sol +0% -0%
array/push/array_push_struct_from_calldata.sol +0% -0%
array/copying/array_copy_including_array.sol +0%
various/destructuring_assignment.sol +0% -0%
array/copying/array_copy_target_simple.sol +0%
functionCall/creation_function_call_with_salt.sol +0%
array/copying/array_copy_storage_storage_different_base_nested.sol +0%
array/copying/array_nested_memory_to_storage.sol +0%
array/copying/nested_array_of_structs_calldata_to_storage.sol +0%
array/copying/elements_of_nested_array_of_structs_calldata_to_storage.sol +0%
structs/copy_struct_array_from_storage.sol +0%
functionCall/creation_function_call_with_args.sol +0%
immutable/multi_creation.sol +0%
types/mapping/copy_from_mapping_to_mapping.sol +0% +0%
array/copying/array_elements_to_mapping.sol +0% +0%
array/fixed_array_cleanup.sol +0%
array/copying/nested_dynamic_array_element_calldata_to_storage.sol +0%
array/copying/nested_array_of_structs_memory_to_storage.sol +0%
array/copying/array_copy_storage_storage_struct.sol +0%
array/copying/array_copy_storage_storage_static_dynamic.sol +0%
abiencodedecode/abi_decode_simple_storage.sol +0% -0%
structs/calldata/calldata_struct_with_nested_array_to_storage.sol +0% +0%
array/bytes_length_member.sol +0%
abiEncoderV1/abi_decode_v2_storage.sol +0% +0%
array/push/array_push_nested_from_calldata.sol +0% -0%
array/copying/array_of_structs_containing_arrays_memory_to_storage.sol +0%
array/copying/nested_array_of_structs_storage_to_storage.sol +0%
array/push/push_no_args_2d.sol +0%
structs/struct_copy.sol +0%
array/copying/array_storage_multi_items_per_slot.sol +0%
array/copying/function_type_array_to_storage.sol +0% -0%
abiEncoderV2/calldata_overlapped_dynamic_arrays.sol +0% -0%
array/copying/copy_function_internal_storage_array.sol +0%
array/copying/array_copy_storage_storage_dyn_dyn.sol +0%
various/staticcall_for_view_and_pure.sol -0%
array/array_storage_length_access.sol -0%
array/copying/copy_byte_array_in_struct_to_storage.sol +0% -0%
calldata/copy_from_calldata_removes_bytes_data.sol -0%
array/copying/array_copy_storage_storage_static_static.sol -0%
array/copying/bytes_inside_mappings.sol -0%
array/invalid_encoding_for_storage_byte_array.sol -0% -0%
array/copying/array_of_function_external_storage_to_storage_dynamic_different_mutability.sol -0% +0%
constructor/no_callvalue_check.sol -0%
array/copying/array_copy_nested_array.sol -0% -0%
array/copying/elements_of_nested_array_of_structs_memory_to_storage.sol -0%
array/array_memory_index_access.sol -0%
array/copying/copy_removes_bytes_data.sol -0%
various/many_subassemblies.sol -0%
array/copying/calldata_array_to_mapping.sol -0%
functionCall/mapping_array_internal_argument.sol -0%
array/copying/array_to_mapping.sol -0% +0%
array/copying/array_of_function_external_storage_to_storage_dynamic.sol -0% -0%
structs/struct_containing_bytes_copy_and_delete.sol -0% -0%
array/copying/array_copy_target_simple_2.sol -0%
abiEncoderV2/storage_array_encoding.sol -0% -0%
structs/copy_substructures_from_mapping.sol -0% -0%
various/skip_dynamic_types_for_structs.sol -0% -0%
storage/empty_nonempty_empty.sol -0% -0%
externalContracts/strings.sol -0% +0%
structs/copy_to_mapping.sol -0% -0%
structs/copy_from_mapping.sol -0% -0%
array/pop/byte_array_pop_long_storage_empty_garbage_ref.sol -0%
array/copying/array_copy_calldata_storage.sol -0% -0%
array/copying/storage_memory_nested_bytes.sol -0% +0%
array/copying/arrays_from_and_to_storage.sol -0%
libraries/internal_types_in_library.sol -0%
functionCall/external_call_to_nonexisting_debugstrings.sol -0% +0%
structs/copy_substructures_to_mapping.sol -0% -0%
functionCall/gas_and_value_brace_syntax.sol -0%
functionCall/gas_and_value_basic.sol -0%
constructor/arrays_in_constructors.sol -1% +0%
structs/structs.sol -0%
array/pop/byte_array_pop_masking_long.sol -0% -0%
libraries/using_library_mappings_return.sol -0%
array/copying/array_copy_target_leftover.sol -0% -0%
array/arrays_complex_from_and_to_storage.sol -1% +0%
functionCall/external_call_to_nonexisting.sol -1% +0%
constructor/bytes_in_constructors_unpacker.sol -1% -0%
externalContracts/deposit_contract.sol -1% +0%
various/address_code.sol -1% -0%
array/function_array_cross_calls.sol -2% 1%
various/create_calldata.sol -1% -0%
externalContracts/ramanujan_pi.sol -6%

@cameel cameel force-pushed the seqbench-sequence-the-good-parts branch from 60b7222 to 54b7ea1 Compare March 6, 2024 13:12
@cameel cameel changed the base branch from develop to seqbench-sequence-the-good-parts-benchmark-base March 6, 2024 13:18
@cameel cameel force-pushed the seqbench-sequence-the-good-parts-benchmark-base branch from 7d6ebb8 to 6beeacb Compare March 6, 2024 13:40
@cameel cameel force-pushed the seqbench-sequence-the-good-parts branch from 54b7ea1 to b453a58 Compare March 6, 2024 15:23
@cameel cameel changed the base branch from seqbench-sequence-the-good-parts-benchmark-base to bump-resource-class-for-ext-tests-failing-due-to-oom March 6, 2024 16:18
@cameel
Copy link
Member Author

cameel commented Mar 6, 2024

External test benchmark diff

CI run on bump-resource-class-for-ext-tests-failing-due-to-oom vs CI run on seqbench-sequence-the-good-parts

ir-no-optimize

project bytecode_size deployment_gas method_gas
brink 0%
colony 0%
elementfi 0%
ens 0%
perpetual-pools 0% -0% -0%
uniswap 0% 0% -0%
yield_liquidator 0% -0% 0%
zeppelin

ir-optimize-evm+yul

project bytecode_size deployment_gas method_gas
brink +0.73% ❌
colony +0.13% ❌
elementfi -1.33% ✅
ens +0.39% ❌ -2.77% ✅ +0.02% ❌
perpetual-pools -0.39% ✅ -0.78% ✅ +0.32% ❌
uniswap +2.06% ❌ +2.2% ❌ +2.13% ❌
yield_liquidator +1.3% ❌ +0.34% ❌ +0.17% ❌
zeppelin -0.2% ✅ -1.16% ✅ +0.12% ❌

ir-optimize-evm-only

project bytecode_size deployment_gas method_gas
brink 0%
colony 0%
elementfi 0%
ens 0% 0% 0%
perpetual-pools 0% -0% -0.01% ✅
uniswap 0% -0% -0%
yield_liquidator 0% -0% -0%
zeppelin 0%

legacy-no-optimize

project bytecode_size deployment_gas method_gas
brink 0%
colony 0%
elementfi 0%
ens 0%
perpetual-pools 0% -0% -0.01% ✅
uniswap 0% +0% +0%
yield_liquidator 0% +0% 0%
zeppelin 0% -0% -0.19% ✅

legacy-optimize-evm+yul

project bytecode_size deployment_gas method_gas
brink -0.03% ✅
colony +0.13% ❌
elementfi +0.12% ❌
ens +0.18% ❌ -0.02% ✅ -0%
perpetual-pools -0.03% ✅ +0% -0.01% ✅
uniswap +0.04% ❌ +0.03% ❌ +0%
yield_liquidator +0.09% ❌ +0.06% ❌ -0.01% ✅
zeppelin +0.15% ❌ +0.17% ❌ -0.02% ✅

legacy-optimize-evm-only

project bytecode_size deployment_gas method_gas
brink 0%
colony 0%
elementfi 0%
ens 0% 0% 0%
perpetual-pools 0% -0% -0%
uniswap 0% -0% +0%
yield_liquidator 0% -0% 0%
zeppelin 0% +0% +0.15% ❌

!V = version mismatch
!B = no value in the "before" version
!A = no value in the "after" version
!T = one or both values were not numeric and could not be compared
-0 = very small negative value rounded to zero
+0 = very small positive value rounded to zer

@cameel
Copy link
Member Author

cameel commented Mar 6, 2024

Comparing with the same table for single-pass, results seem mostly positive, and better overall, though still not consistently better than develop.

  • The two previous outliers in terms of bytecode size got more in line, while uniswap became a smaller outlier:
    • elementfi bytecode size: -3.59% ✅ -> -1.33% ✅
    • ens bytecode size: +4.55% ❌ -> +0.39% ❌
    • uniswap bytecode size: +1.18% ❌ -> +2.06% ❌
    • uniswap method gas: +0.12% ❌ -> +2.13% ❌
  • Things got slightly better in terms of bytecode size for some other projects:
    • zeppelin bytecode size: +0.54% ❌ -> -0.2% ✅
    • brink bytecode size: +1.32% ❌ -> +0.73% ❌
    • yield-liquidator bytecode size: +1.67% ❌ -> +1.3% ❌
  • Other bytecode size changes are minimal and both positive and negative.
  • Aside from uniswap, method gas shifted by up to 0.15%, both positively and negatively.
  • The effect on legacy pipeline got smaller overall. The differences were in the range of -0.3%..+0.3% before, now -0.2%..+0.02%

Note that in either case these values do not take into account the effects of #14854 or #14907. They're both diffing against the current develop.

@cameel
Copy link
Member Author

cameel commented Mar 6, 2024

Compilation times of external tests

Test baseline (#14915) this PR Change
t_native_test_ext_zeppelin 4:42 2:31 -46%
t_native_test_ext_yield_liquidator 0:34 0:26 -24%
t_native_test_ext_uniswap 1:09 0:44 -36%
t_native_test_ext_perpetual_pools 1:01 0:48 -21%
t_native_test_ext_ens 0:39 0:30 -23%
t_native_test_ext_elementfi 2:37 1:56 -26%
t_native_test_ext_brink 0:07 0:06 -14%
t_ems_compile_ext_colony 2:26 1:51 -24%

Comparing with timing for single-pass, results for specific tests changed, but they're all overall in the same ballpark of 15-45% improvement.

Note that the baseline has significant variance. For some tests it's different even up to 25% compared with results for single-pass.

As another data point: the runtime of bytecode comparison for optimized IR decreased from 8:56 to 6:47 (-24%).

@cameel
Copy link
Member Author

cameel commented Mar 6, 2024

I ran my seqbench on erc20.sol and looks like the 4% gas increase is coming mostly from a ~65 byte difference in bytecode size:

runtime gas before runtime gas after bytecode size before bytecode size after
default 323,363 309,700 (-4.2%) 2,288 1,442 (-37.0%)
the-good-parts 323,363 309,956 (-4.1%) 2,288 1,505 (-34.2%)

Note that it's less than 4% here, because gas_diff_stats.py compares with the values achieved by the default sequence (since these are the ones we have in the repo) and they're smaller than the unoptimized values that seqbench takes as a baseline.

Here's how it looks like over the whole run:

bytecode-size
runtime-gas

@cameel cameel force-pushed the seqbench-sequence-the-good-parts branch from b453a58 to 22c8ede Compare March 7, 2024 17:57
@cameel cameel changed the base branch from bump-resource-class-for-ext-tests-failing-due-to-oom to fix-uniswap-perpetual-pools March 7, 2024 17:58
Base automatically changed from fix-uniswap-perpetual-pools to develop March 7, 2024 18:32
@cameel cameel force-pushed the seqbench-sequence-the-good-parts branch from 22c8ede to 3c3fbaa Compare March 7, 2024 18:59
@cameel
Copy link
Member Author

cameel commented Mar 7, 2024

Extra results from bytecode size benchmarks that were re-enabled by #14916.

Overall, everything seems in the same overall range as in previous benchmark so it does not change the final conclusion.

ir-optimize-evm+yul

project bytecode_size deployment_gas method_gas
bleeps
chainlink -1.83% ✅
euler +1.29% ❌
gnosis
gp2 +0.86% ❌
pool-together -0.83% ✅
trident +0.4% ❌

legacy-optimize-evm+yul

project bytecode_size deployment_gas method_gas
bleeps +0.13% ❌
chainlink +0.13% ❌
euler +0.17% ❌
gnosis +0.2% ❌
gp2 +0.27% ❌
pool-together +0.05% ❌
trident +0.1% ❌

@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Mar 22, 2024
@ethereum ethereum deleted a comment from github-actions bot Mar 25, 2024
@cameel cameel removed the stale The issue/PR was marked as stale because it has been open for too long. label Mar 25, 2024
@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Apr 9, 2024
@cameel cameel removed the stale The issue/PR was marked as stale because it has been open for too long. label Apr 9, 2024
@ethereum ethereum deleted a comment from github-actions bot Apr 9, 2024
@cameel cameel closed this Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant