On Mon, May 30, 2016 at 11:54:45AM +0100, Viktor Bachraty wrote: > Hi Iustin, > > Thanks for helping out with the optimizations. AFAIK, Klaus had a plan to > introduce a type for UUIDs (that could have been backed by a ByteString) > instead of using just a String before the project was handed over to us.
OK, good to have confirmation that this was planned, it means there are no obvious known issues with it. > If > this could be done in a not too invasive change and it would reduce > allocations as well, that would be awesome. Cool, will do then. I'll see how invasive it gets and I'll send it against stable-2.16 (if not too invasive) or master otherwise. iustin > On Mon, May 30, 2016 at 11:40 AM, 'Iustin Pop' via ganeti-devel < > [email protected]> wrote: > > > On Thu, May 26, 2016 at 03:56:10PM +0100, Ganeti Development List wrote: > > > This is exercised by the luxi QueryInstances call when the sinst_cnt and > > > sint_list fields are used. This uses a lot of CPU and does a lot of > > > short-lived heap allocation on clusters with many instances. > > > > > > The reimplementation allocates fewer temporary values, and does fewer > > > object lookups by UUID. The net effect is to reduce heap use from ~3.2GB > > > to ~1.5GB, and CPU use from ~1200ms to ~770ms in a test harness using > > > a config with 1000 DRBD instances on 80 nodes. > > > > Hi Brian & all, > > > > I was surprised to see that much heap use and wall time for this code, > > so I looked at this yesterday (thanks for the config file/test > > hardness!). > > > > Profiling shows that overall, the allocation for this test harness is > > split half-half between config loading (which is a separate issue) and > > the UTF8.fromString calls for converting between the disk IDs (stored as > > String in the instance) and the keys for configDisks (which are > > ByteStrings). > > > > Writing a simple 20-line hack to change the instance disks to be > > ByteStrings shows that the runtime of just "map (snd . getNodeInstances > > cfg) all_nodes" goes from (on my machine) 200ms to ~45 ms., i.e. a 5× > > decrease in runtime, with the getNodeInstances doing very few > > allocations. > > > > Is there a reason not to store UUIDs everywhere as ByteStrings? My > > quick and dirty patch seems to pass all unittests. > > > > Or alternatively: is anyone already working on making UUIDs uniform? If > > not, I'll work on making my patch clean and ready for submit. > > > > thanks, > > iustin > >
