codebase: added codegen-units to release profile

[KarthikNedunchezhiyan]

Reason for This PR

"codegen-units = 1" improves the performance of the code. check
https://doc.rust-lang.org/rustc/codegen-options/index.html#codegen-units

Note: It increases the compile time due to non-parallel processing in LLVM.

Description of Changes

• Added "codegen-units = 1" in release profile.

• This functionality can be added in rust-vmm.

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license.

PR Checklist

[Author TODO: Meet these criteria.]
[Reviewer TODO: Verify that these criteria are met. Request changes if not]

• All commits in this PR are signed (git commit -s).
• The reason for this PR is clearly provided (issue no. or explanation).
• The description of changes is clear and encompassing.
• Any required documentation changes (code and docs) are included in this PR.
• Any newly added unsafe code is properly documented.
• Any API changes are reflected in firecracker/swagger.yaml.
• Any user-facing changes are mentioned in CHANGELOG.md.
• All added/changed functionality is tested.

[serban300]

Hi @KarthikNedunchezhiyan ! Thanks for the contribution !

Did you try to do any performance tests with this change ? Do you know what would be the improvement ?

[KarthikNedunchezhiyan]

@serban300 I did'nt got time to run performance test. But from the documentation LLVM is generating parallel code block to improve compilation speed. By setting this option to 1, LLVM will perform code compilation linearly by which better placement of instructions to improve code performance.

[dianpopa]

Hi @KarthikNedunchezhiyan

We have not experimented with Rust's "codegen units" until now, however your proposal sounds interesting to me. Before giving any advice here, I would like to have a discussion inside the team to talk more about the implications of codegen-units. We will keep you posted.

PS: The CI fail seems to come from the fact that the build on arm takes longer than the implicit timeout of 300 sec (we kind of expected that since the build takes longer with codegen units enabled. We ll come back with a proposal there too.

Thanks,
Diana

[serban300]

Hi @KarthikNedunchezhiyan ! We're currently working on a performance tests framework. After it's done, we'll run the performance tests on this PR in order to get a better idea on the impact of this change.

[gbionescu]

I just wanted to note that I ran a smoke test recently using this change on an x86 machine, but didn't see any significant differences when running a network intensive workload. I did see that the binary size was about 100kb smaller.

There may be some differences here, but these differences may be so small that it would be hard to detect it.

[AlexandruCihodaru]

Hello @KarthikNedunchezhiyan, thank you for your contribution. We have ran our performance tests suit with the changes proposed by you. After aggregating the results we noticed that there is no significant performance improvement,(i.e the increase is on average 0.17% for vsock device and 0.81% for network tcp throughput). However for the block device there was a greater improvement of 10% in average. To summarize, while there was a small performance increase the build time increase significantly as such we have decided to close this PR for the moment. However we are not entirely excluding the possibility of using codegen in the future. As per our roadmap, this year we will be working towards improving IO performance of Firecracker. It is possible that the technologies we will deploy here (e.g., we're looking into io_uring, interrupt suppression) will interact with codegen behaviour, so we are planing on testing this out again after we merge those. Let us know if that doesn't make sense & thank you for prompting us to take a deep look here