Skip to content

fix clippy slow_vector_initialization #618

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

klensy
Copy link
Contributor

@klensy klensy commented May 4, 2024

Should be smaller code size

Should be smaller codegen size
@klensy
Copy link
Contributor Author

klensy commented May 4, 2024

Ahh, this should affect #[cfg(any(target_arch = "x86", target_arch = "arm"))] only.

@klensy
Copy link
Contributor Author

klensy commented May 4, 2024

Why CI ignores changes in workflow? idk.
Looks like it uses workflow from master.

@klensy klensy force-pushed the zeroed branch 2 times, most recently from 1eb5e0e to 9234bc7 Compare May 4, 2024 19:14
@@ -409,8 +409,7 @@ pub fn init() -> Result<Init, ()> {
//
// See https://learn.microsoft.com/cpp/build/reference/pdbpath for an
// example of where symbols are usually searched for.
let mut search_path_buf = Vec::new();
search_path_buf.resize(1024, 0);
let mut search_path_buf = vec![0; 1024];
Copy link
Member

Choose a reason for hiding this comment

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

Hmm. This could be with_capacity, and then we pass the search_path_buf.capacity() when indicating how much space we have to allocate, and then we pass a search_path_buf.set_len after filling it.

...or that could be over-optimization as we are likely to get a fresh raw page from the OS because this was a calloc.

Copy link
Member

Choose a reason for hiding this comment

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

fresh pages from the OS may still be more expensive than a recycled allocation from a pooling allocator. But I'd only play those kinds of games if this shows up in a benchmark and not just clippy linting about it.

Copy link

Code size changes for a hello-world Rust program linked with libstd with backtrace:

On platform windows-latest:

  • Original binary size: 133,120 B
  • Updated binary size: 134,656 B
  • Difference: 1,536 B (1.15%)

@workingjubilee
Copy link
Member

@klensy apparently needs imports?

Copy link

Code size changes for a hello-world Rust program linked with libstd with backtrace:

On platform windows-latest:

  • Original binary size: 133,120 B
  • Updated binary size: 134,656 B
  • Difference: 1,536 B (1.15%)

@klensy
Copy link
Contributor Author

klensy commented Sep 8, 2024

https://github.com/actions/upload-artifact/blob/main/docs/MIGRATION.md
18efcf9

Looks like upload-artifact should be bumped too

@workingjubilee
Copy link
Member

It was in ...

Oh we bumped download not upload.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants