"Dr. David Alan Gilbert" <dgilb...@redhat.com> wrote: > * Juan Quintela (quint...@redhat.com) wrote: >> We were calling qemu_target_page_size() left and right. >> >> Signed-off-by: Juan Quintela <quint...@redhat.com>
[Adding Richard] > (Copying in Peter Maydell) > Your problem here is most of these files are target independent > so you end up calling the qemu_target_page_size functions, which I guess > you're seeing popup in some perf trace? > I mean they're trivial functions but I guess you do get the function > call. Hi There are several problems here: - Richard complained in previous reviews that we were calling qemu_target_page_size() inside loops or more than once per function (He was right) - qemu_target_page_size() name is so long that basically means that I had to split the line for each appearance. - All migration code assumes that the value is constant for a current migration, it can change. So I decided to cache the value in the structure and call it a day. The same for the other page_count field. I have never seen that function on a performance profile, so this is just a taste/aesthetic issue. I think your patch is still good, but it don't cover any of the issues that I just listed. Thanks, Juan. > > I wonder about the following patch instead > (Note i've removed the const on the structure here); I wonder how this > does performance wise for everyone: > > > From abc7da46736b18b6138868ccc0b11901169e1dfd Mon Sep 17 00:00:00 2001 > From: "Dr. David Alan Gilbert" <dgilb...@redhat.com> > Date: Mon, 16 May 2022 19:54:31 +0100 > Subject: [PATCH] target-page: Maintain target_page variable even for > non-variable > Content-type: text/plain > > On architectures that define TARGET_PAGE_BITS_VARY, the 'target_page' > structure gets filled in at run time by the number of bits and the > TARGET_PAGE_BITS and TARGET_PAGE macros use that rather than being > constant. > > On non-variable pagesize systems target_page is not filled in, and we > rely on TARGET_PAGE_SIZE being compile time defined. > > The problem is that for source files that are target-independent > they end up calling qemu_target_page_size to read the size, and that > function call is annoying. > > Improve this by always filling in 'target_page' even for non-variable > size CPUs, and inlining the functions that previously returned > the macro values (that may have been constant) to return the > values read from target_page. > > Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
