debug codspeed build on main

[davidhewitt]

For understanding #705 - I want to see what the build flags look like when building this branch versus that one, to identify the cause of the benchmark change.

[samuelcolvin]

Cc @art049 who might have some ideas.

Also worth running the benchmarks locally and seeing if there's any change.

[codecov-commenter]

Codecov Report

Merging #707 (c86b615) into main (424a2f8) will not change coverage.
The diff coverage is n/a.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #707   +/-   ##
=======================================
  Coverage   93.73%   93.73%           
=======================================
  Files          99       99           
  Lines       13807    13807           
  Branches       25       25           
=======================================
  Hits        12942    12942           
  Misses        859      859           
  Partials        6        6           

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 424a2f8...c86b615. Read the comment docs.

[codspeed-hq]

CodSpeed Performance Report

Merging #707 dh/codspeed-debug (f91a44c) will improve performances by 43.61%.

Summary

🔥 38 improvements
❌ 0 regressions
✅ 87 untouched benchmarks

🆕 0 new benchmarks
⁉️ 0 dropped benchmarks

Benchmarks breakdown

	Benchmark	main	dh/codspeed-debug	Change
🔥	test_complete_core_lax	1.5 ms	1.4 ms	+11.5%
🔥	test_core_python_fs	410.8 µs	348.7 µs	+17.8%
🔥	test_core_python	561.7 µs	508.1 µs	+10.55%
🔥	test_list_of_dict_models_core	565.3 µs	513.7 µs	+10.05%
🔥	test_list_of_ints_core_py	3.1 ms	2.6 ms	+19.99%
🔥	test_set_of_ints_core_duplicates	2.8 ms	2.4 ms	+18.86%
🔥	test_frozenset_of_ints_core	1.5 ms	1.3 ms	+11.38%
🔥	test_frozenset_of_ints_duplicates_core	1,184.3 µs	998.3 µs	+18.63%
🔥	test_dict_of_ints_core	6.8 ms	6 ms	+12.8%
🔥	test_dict_of_any_core	4.9 ms	4.2 ms	+16.19%
🔥	test_many_models_core_dict	6.2 ms	5.5 ms	+13.14%
🔥	test_many_models_core_model	16.3 ms	14.7 ms	+10.49%
🔥	test_list_of_nullable_core	809.6 µs	640.7 µs	+26.36%
🔥	test_core_str	31 µs	27.7 µs	+11.83%
🔥	test_core_future_str	31.7 µs	28.4 µs	+11.55%
🔥	test_tuple_many_variable	36.5 µs	32.5 µs	+12.29%
🔥	test_arguments	48.1 µs	43.3 µs	+10.97%
🔥	test_generator_rust	33.3 µs	29.6 µs	+12.68%
🔥	test_definition_in_tree	7.3 ms	6 ms	+21.11%
🔥	test_definition_out_of_tree	11.2 ms	9.9 ms	+13.34%
🔥	test_core_root_model	130.5 µs	110.8 µs	+17.69%
🔥	test_core_dict	380 µs	286.1 µs	+32.82%
🔥	test_core_dict_filter	390.4 µs	296.1 µs	+31.86%
🔥	test_core_json	453.9 µs	396.1 µs	+14.58%
🔥	test_json_direct_list_str	1,086.9 µs	928.5 µs	+17.05%
🔥	test_python_json_list_str	1,006.7 µs	710 µs	+41.79%
🔥	test_json_any_list_str	1.8 ms	1.6 ms	+10.9%
🔥	test_json_direct_list_int	1,067.6 µs	904.9 µs	+17.98%
🔥	test_json_any_list_int	1.7 ms	1.6 ms	+11.51%
🔥	test_python_json_list_int	1,003.9 µs	706.8 µs	+42.02%
🔥	test_python_json_list_none	984.7 µs	685.7 µs	+43.61%
🔥	test_core_model_py	58.8 µs	51.8 µs	+13.5%
🔥	test_core_model_py_extra	68.5 µs	60.1 µs	+14%
🔥	test_core_model_json	53.4 µs	46.4 µs	+15.1%
🔥	test_core_model_json_extra	63.7 µs	56.6 µs	+12.52%
🔥	test_model_exclude_unset_false	62.1 µs	55.8 µs	+11.42%
🔥	test_model_exclude_unset_true	76 µs	67.7 µs	+12.2%
🔥	test_model_list_core_json	1,131.3 µs	968.4 µs	+16.82%

[davidhewitt]

So having got my head around this, my hunch is that #662 introducing make build-prod actually didn't disable use of mimalloc properly. I'm going to remove that line here and retest.

EDIT: no, that doesn't seem to be the case from further digging.

[davidhewitt]

Aha, I've found it in the last commit.

maturin builds the production binary with cargo rustc --crate-type=cdylib, which passes -C lto=fat to the final compilation. This isn't the case when using cargo build --release, potentially because the rlib format isn't a final binary and can't have LTO applied. Maybe it's a cargo bug.

Closing this as I now know what I need.

[art049]

hey @davidhewitt, by any chance did you manage to understand what could have changed in the benches? maybe some different flags? I'm interested to try to understand this change!

[davidhewitt]

Yes, this looks like the cause of the change:

maturin builds the production binary with cargo rustc --crate-type=cdylib, which passes -C lto=fat to the final compilation. This isn't the case when using cargo build --release, potentially because the rlib format isn't a final binary and can't have LTO applied. Maybe it's a cargo bug.