Skip to content

Remove redundant zeroing #95

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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

danielzgtg
Copy link

@danielzgtg danielzgtg commented Apr 15, 2025

std::vector::resize already zeroes the new samples, so we don't need to use std::fill. See https://stackoverflow.com/a/73333009/10477326

However, we pass a second parameter to explicitly copy-init with zero
and to not default-init. Benchmarks showed the performance difference
between the two to be statistically insignificant, despite default-init
having memset.

@adamstark adamstark changed the base branch from master to develop May 30, 2025 21:59
@adamstark
Copy link
Owner

Hi there, thanks for this! I think it depends slightly because the type of the vector depends on the class template type T, which is very very likely to be a float or double, or an integer of some type, but in theory one could use it with a different parameter for T (as long as that parameter fulfilled the requirements for being assigned values etc).

Perhaps the cleaner way to do this is to use the second parameter of resize? I.e.

samples[i].resize (numSamples, static_cast (0));

just to explicitly make clear we want to zero-initialise any new samples?

@danielzgtg danielzgtg force-pushed the feat/redundant-zeroing branch 2 times, most recently from e5eb963 to 30bbc55 Compare May 31, 2025 00:58
@danielzgtg
Copy link
Author

@adamstark Fixed

Despite my initial assumption default-init would be faster because of memset, benchmarking your method showed no difference.

std::vector::resize already zeroes the new samples, so we don't need
to use std::fill. See https://stackoverflow.com/a/73333009/10477326

However, we pass a second parameter to explicitly copy-init with zero
and to not default-init. Benchmarks showed the performance difference
between the two to be statistically insignificant, despite default-init
having memset.
@danielzgtg danielzgtg force-pushed the feat/redundant-zeroing branch from 30bbc55 to 3d5f585 Compare May 31, 2025 01:04
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.

2 participants