Speedup MultiFab registry#6139
Conversation
|
Wow, good catch!! Do we maybe want to implement a faster to-string routine in AMReX for |
|
Interesting! |
|
This might achieve the same caching, but upstream :) |
|
Just curious, could the key type of the m_mf_register map be changed from string to the enum type (int)? Or is the access by string still more common? |
Access using the enum and string are needed. In some cases the same MF is accessed with the enum and string so it needs to be consistent. There are a number of MFs where the name is generated dynamically (for instance to include the species name). |
## Summary The `getEnumNameValuePairs` logic used below a lot of `AMREX_ENUM` functions is very slow for long-enough (e.g., N>70) enums. In WarpX 1D simulations, it took up close to 50% of the runtime, as Dave Grote found. This caches the key-value string list on first creation, because the type is known at compile-time. A better implementation might skip the dictionary altogether for functions that just need one string, like `getEnumNameString`. ## Additional background BLAST-WarpX/warpx#6139
|
@ax3l While the recent commit to AMReX (AMReX-Codes/amrex#4643) improves things, it doesn't seem to be as big of an improvement compared to the changes in this PR. Using the 1D uniform plasma test that I have showed during KISMET meetings, here are the wall times: I've ran the tests several times and get the same result - This PR is about 10% faster. |
|
Could you try this? AMReX-Codes/amrex#4654 |
That seems to work just as good as this PR. |
|
Fixed upstream via AMReX-Codes/amrex#4654 |
|
Thank you @dpgrote @JustinRayAngus and @WeiqunZhang for identifying, testing and fixing this! |
|
@JustinRayAngus @dpgrote if you have the 1D benchmark you used available and could add it to the WarpX CI tests, I would be curious to also benchmark it at some point :) |
This PR is a possible solution to speed up the MultiFab registry, when getting MultiFabs.
Most MultiFabs in WarpX are specified by the enum FieldType, and when the MultiFab is fetched (using
get), the enum is converted to a string since the MultiFab map keys are strings. This conversion is done using theamrex::getEnumNameStringroutine. That routine then internally callsamrex::getEnumNameValuePairswhich generates a vector of the name, value pairs for whole enum type. ThengetEnumNameStringreturns the one name requested. This means that every time agetis done, the whole name, value vector for the enum is generated.The
FieldTypeenum has 67 values, so creating that vector is somewhat expensive. For 2D and 3D, this time is not significant, but for 1D it can be. In a test case, this was taking up about half of the run time (since it was done almost 30 million times).There are two relatively simple solutions. The first is to not use the
FieldTypeenum, but always use strings for the MultiFabs. A second solution is to cache the names. This PR shows what that might look like.This cache is not perfect. This assumes that the int values of each of the enums is unique. This is true in WarpX, but some other code could break this (by using more than one enum type for the MultiFabs for example).