Skip to content

Replace to emplace C++11 for code refactor and minor optimize inserts #1875

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

Merged
merged 1 commit into from
Apr 22, 2025

Conversation

GermanAizek
Copy link
Contributor

@GermanAizek GermanAizek commented Nov 3, 2024

Rather, it refers more to refactoring and code reduction, I checked assembly listing -O0 and -O2 on clang. And it also affects optimization insert on Release configuration.

Clang 19 emplace -O0
изображение

Clang 19 emplace -O2
изображение

VS

Clang 19 insert make_pair -O0
изображение

Clang 19 insert make_pair -O2
изображение

Copy link
Contributor

@MicroYY MicroYY left a comment

Choose a reason for hiding this comment

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

Pls fix the compilation error

@GermanAizek
Copy link
Contributor Author

Pls fix the compilation error

fix it c95e85f

@MicroYY
Copy link
Contributor

MicroYY commented Nov 5, 2024

Thanks for your patching.
I understand emplace is more efficient than insert because emplace reduces extra object copy, so you proposed this PR. One question for my curiosity, didn't you meet perf issues bottlenecked by the STL container operations? Or the PR is proposed out of motivations to make code better?

@GermanAizek
Copy link
Contributor Author

GermanAizek commented Nov 5, 2024

Thanks for your patching. I understand emplace is more efficient than insert because emplace reduces extra object copy, so you proposed this PR. One question for my curiosity, didn't you meet perf issues bottlenecked by the STL container operations? Or the PR is proposed out of motivations to make code better?

@MicroYY
It's more like code refactoring, optimization is insignificant, but there is, better pay ur attention to 2 previous PR where code generation is really optimized and media-driver can save time on pre-reserved memory.

reserve vector optimization insert: #1869

And there is also very good optimization classes and structures that are often moved and copied, optimization affects all modern x64 processors.

align for x64 cpu: #1873

@GermanAizek
Copy link
Contributor Author

@MicroYY maybe I split commit into many commits? Code review takes a long time.

@MicroYY MicroYY added the verifying PR: fix ready and verifying with build/test label Nov 14, 2024
@MicroYY MicroYY removed the verifying PR: fix ready and verifying with build/test label Jan 8, 2025
@MicroYY
Copy link
Contributor

MicroYY commented Jan 8, 2025

The merge process is blocked due to internal merging conflict. Could you pls rebase this pull request?

@GermanAizek
Copy link
Contributor Author

The merge process is blocked due to internal merging conflict. Could you pls rebase this pull request?

ok, wait.

@GermanAizek
Copy link
Contributor Author

@MicroYY, fix it here 5232038

@MicroYY MicroYY added the verifying PR: fix ready and verifying with build/test label Jan 14, 2025
@MicroYY
Copy link
Contributor

MicroYY commented Jan 16, 2025

Hi @GermanAizek
Due to the limitation of merging process, pls kindly rebase the 4 commits into 1. Otherwise the merging will be blocked...
And you need to rebase again due to some new conflict since this change touches lots of files..
Or could you grant me the write access to your forked repo? Then I can help modify your commit

@MicroYY
Copy link
Contributor

MicroYY commented Apr 21, 2025

Hi @GermanAizek
There are 5 commits in this pull request. Pls kindly merge the 4 commits into 1, Otherwise the merging will be blocked...

@GermanAizek
Copy link
Contributor Author

GermanAizek commented Apr 21, 2025

@MicroYY,
I squash all commits, done.

@GermanAizek GermanAizek force-pushed the master branch 2 times, most recently from 0141fd2 to 51f9fe1 Compare April 21, 2025 13:16
Signed-off-by: Herman Semenov <GermanAizek@yandex.ru>
@intel-mediadev intel-mediadev merged commit 67520e6 into intel:master Apr 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
verifying PR: fix ready and verifying with build/test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants