Skip to content
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

Improve performance #53

Open
3 tasks
mihaisebea opened this issue Aug 30, 2020 · 2 comments
Open
3 tasks

Improve performance #53

mihaisebea opened this issue Aug 30, 2020 · 2 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@mihaisebea
Copy link

  • look into a faster undordered map - for big files most of the time is spent on hashing and looking up in maps

  • use a thread pool to do all of the exporting in paralel

  • maybe use std terminate to end program execution earlier so we don't have to wait for all those strings and maps to be freed?

@MetanoKid
Copy link
Owner

Thank you for the feedback, Mihai!

It's true I hadn't looked into performance before releasing v1.0.0, so all of your points make total sense.
Let me comment on them:

  • look into a faster undordered map - for big files most of the time is spent on hashing and looking up in maps

I ran Visual Studio 2017's profiler on a Release version analyzing a 3.3 GB trace collected when compiling Unreal Engine 4.23 in my old laptop (i5 2th gen) using VS2017 (i.e. no templates recorded). These were the most hit functions:

unreal-engine-profile

In my case, hasing and look up don't seem like an issue. Calling UndecorateFunction, however, looks bad. We should look into that, will probably create a new issue to address it.
I'm not proficient in other (faster) maps so I'm totally open to seeing some performance comparisons.

  • use a thread pool to do all of the exporting in paralel

Totally on point. I wanted to do this some time ago but it got down into the backlog.

  • maybe use std terminate to end program execution earlier so we don't have to wait for all those strings and maps to be freed?

In fact, waiting for all objects to be destructed was a big issue for me in my old laptop. That same 3.3 GB trace file I mentioned earlier would take over 30 minutes to terminate after the last log was shown. That was until cc71025, where filters get applied at an earlier point and less TimelineEntry instances are created.
I'm against aborting execution even in the last line of main, however. Yes, we could rely on the OS to collect the memory we were using. And yes, we're currently sure we don't need to wait on anything to keep a clean state.
I don't know whether this certainty will change in the future, but it doesn't feel right to me at this point.
Just to give an option, we could add a command-line switch that enables calling std::terminate before returning main (in that case, we must be sure we don't alter the return code so a valid build doesn't set an error level).

@MetanoKid MetanoKid added enhancement New feature or request help wanted Extra attention is needed labels Aug 31, 2020
@mihaisebea
Copy link
Author

I guess my case is a bit more pathological and hashing does show up.

image

About terminate it's just a suggestion for now.
I'll think of some other ways. I need to check where are the allocations are coming from. If we can reduce the number of allocations that time will go down.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants