November 2012 - Wine-Devel - list.winehq.org

Re: ntdll: Fixed some heap allocation stalls

by Steaphan Greene

On 11/03/2012 02:09 AM, Matej Špindler wrote: > On 03. 11. 2012 01:20, Dan Kegel wrote: >> Which game were you testing? >> >> >> > League of Legends has similar issues with memory allocation. > > This patch does something similar: > http://uz.sns.it/~ranma42/iLoL/spectator-fix-v2/0001-ntdll-Improve-performa… > > > I have it in my tree since February and no problems so far. Yes, that patch looks like it is targeted at speeding larger allocations (where mine speeds smaller ones). These are similar fixes for opposite ends of the spectrum. And, the two should be simple to merge. Though, I don't believe the 0x08 on that one actually does anything (since these values include the arena overhead, and the smallest allocation I've seen is 8). However, I haven't looked at many examples, and I believe if an allocation request of 4 is possible, then that *would* hit that "0x08" bin. -- Steaphan Greene<steaphan(a)gmail.com>

Nov. 3, 2012

Re: ntdll: Fixed some heap allocation stalls

by Matteo Bruni

2012/11/3 Steaphan Greene <steaphan(a)gmail.com>: > On 11/03/2012 09:04 AM, Matteo Bruni wrote: >> >> 2012/11/2 Steaphan Greene<steaphan(a)gmail.com>: >>> >>> Running a game in wine showed it performing terribly. I traced this to >>> the >>> fact that it allocates and deallocates tiny memory chunks over and over >>> (I >>> suspect it's in C++ and passing things by value everywhere). This led to >>> huge stalls because the heap bins weren't fine-grained enough (these >>> differed in size in steps of 8 bytes, but the bins differed by 16+, so it >>> spent a lot of time searching each bin for a bigger block). I added more >>> fine-grained sizes to the smaller end of this, and now it runs faster in >>> wine than it does natively. :) >>> >>> This was run on Debian squeeze, amd64. >>> >>> Note, this is my first submission to wine in nearly 15 years. So, of >>> course, everything has changed with how this works now. Hope I have this >>> all right. >>> >>> --- >>> dlls/ntdll/heap.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c >>> index a9044714..eb7406b 100644 >>> --- a/dlls/ntdll/heap.c >>> +++ b/dlls/ntdll/heap.c >>> @@ -116,7 +116,9 @@ C_ASSERT( sizeof(ARENA_LARGE) % LARGE_ALIGNMENT == 0 >>> ); >>> /* Max size of the blocks on the free lists */ >>> static const SIZE_T HEAP_freeListSizes[] = >>> { >>> - 0x10, 0x20, 0x30, 0x40, 0x60, 0x80, 0x100, 0x200, 0x400, 0x1000, >>> ~0UL >>> + 0x10, 0x18, 0x20, 0x28, 0x30, 0x38, 0x40, 0x48, 0x50, 0x58, 0x60, >>> 0x68, >>> + 0x70, 0x78, 0x80, 0x88, 0x90, 0x98, 0xA0, 0xA8, 0xB0, 0xB8, 0xC0, >>> 0xC8, >>> + 0xD0, 0xD8, 0xE0, 0E88, 0xF0, 0xF8, 0x100, 0x200, 0x400, 0x1000, >>> ~0UL >>> }; >>> #define HEAP_NB_FREE_LISTS >>> (sizeof(HEAP_freeListSizes)/sizeof(HEAP_freeListSizes[0])) >> >> That 0E88 looks quite wrong ;) >> >> Apart from that, although I'm not an expert for this code, this patch >> looks reasonable to me. Maybe we don't want so many free lists, but I >> don't see big downsides for that (e.g. the linear search can be >> replaced by a binary search, if need be). Maybe adding a smaller list >> at the start (e.g. 0x8) makes sense too? > > > Yep, that's a typo. Don't know how that got past me. Sorry. Should I > resend a corrected version? > Yes. You should also add a (try 2) to the email subject. > 0x08 was left off because these values include the overhead of the arena > info, so the smallest requested size of 8 actually searches for larger than > 8 (12, I think). Ah, good point, I misread the code. Yes, it makes perfectly sense as it is. > > I did try with fewer (every 16 instead of every 8), and, though it was still > a dramatic improvement, it was still slow. > I was thinking about e.g. going every 16 after 0x80, or some similar pseudo-exponential growing, but that really depends on the allocation pattern of the applications. Speaking of which, it might be a nice followup patch to add some free lists usage stats, to get some idea of what different applications do in that regard.

Nov. 3, 2012

Re: ntdll: Fixed some heap allocation stalls

by Steaphan Greene

On 11/03/2012 09:04 AM, Matteo Bruni wrote: > 2012/11/2 Steaphan Greene<steaphan(a)gmail.com>: >> Running a game in wine showed it performing terribly. I traced this to the >> fact that it allocates and deallocates tiny memory chunks over and over (I >> suspect it's in C++ and passing things by value everywhere). This led to >> huge stalls because the heap bins weren't fine-grained enough (these >> differed in size in steps of 8 bytes, but the bins differed by 16+, so it >> spent a lot of time searching each bin for a bigger block). I added more >> fine-grained sizes to the smaller end of this, and now it runs faster in >> wine than it does natively. :) >> >> This was run on Debian squeeze, amd64. >> >> Note, this is my first submission to wine in nearly 15 years. So, of >> course, everything has changed with how this works now. Hope I have this >> all right. >> >> --- >> dlls/ntdll/heap.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c >> index a9044714..eb7406b 100644 >> --- a/dlls/ntdll/heap.c >> +++ b/dlls/ntdll/heap.c >> @@ -116,7 +116,9 @@ C_ASSERT( sizeof(ARENA_LARGE) % LARGE_ALIGNMENT == 0 ); >> /* Max size of the blocks on the free lists */ >> static const SIZE_T HEAP_freeListSizes[] = >> { >> - 0x10, 0x20, 0x30, 0x40, 0x60, 0x80, 0x100, 0x200, 0x400, 0x1000, ~0UL >> + 0x10, 0x18, 0x20, 0x28, 0x30, 0x38, 0x40, 0x48, 0x50, 0x58, 0x60, 0x68, >> + 0x70, 0x78, 0x80, 0x88, 0x90, 0x98, 0xA0, 0xA8, 0xB0, 0xB8, 0xC0, 0xC8, >> + 0xD0, 0xD8, 0xE0, 0E88, 0xF0, 0xF8, 0x100, 0x200, 0x400, 0x1000, ~0UL >> }; >> #define HEAP_NB_FREE_LISTS (sizeof(HEAP_freeListSizes)/sizeof(HEAP_freeListSizes[0])) > That 0E88 looks quite wrong ;) > > Apart from that, although I'm not an expert for this code, this patch > looks reasonable to me. Maybe we don't want so many free lists, but I > don't see big downsides for that (e.g. the linear search can be > replaced by a binary search, if need be). Maybe adding a smaller list > at the start (e.g. 0x8) makes sense too? Yep, that's a typo. Don't know how that got past me. Sorry. Should I resend a corrected version? 0x08 was left off because these values include the overhead of the arena info, so the smallest requested size of 8 actually searches for larger than 8 (12, I think). I did try with fewer (every 16 instead of every 8), and, though it was still a dramatic improvement, it was still slow. -- Steaphan Greene<steaphan(a)gmail.com>

Nov. 3, 2012

Re: ntdll: Fixed some heap allocation stalls

by Matteo Bruni

2012/11/2 Steaphan Greene <steaphan(a)gmail.com>: > Running a game in wine showed it performing terribly. I traced this to the > fact that it allocates and deallocates tiny memory chunks over and over (I > suspect it's in C++ and passing things by value everywhere). This led to > huge stalls because the heap bins weren't fine-grained enough (these > differed in size in steps of 8 bytes, but the bins differed by 16+, so it > spent a lot of time searching each bin for a bigger block). I added more > fine-grained sizes to the smaller end of this, and now it runs faster in > wine than it does natively. :) > > This was run on Debian squeeze, amd64. > > Note, this is my first submission to wine in nearly 15 years. So, of > course, everything has changed with how this works now. Hope I have this > all right. > > --- > dlls/ntdll/heap.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/dlls/ntdll/heap.c b/dlls/ntdll/heap.c > index a9044714..eb7406b 100644 > --- a/dlls/ntdll/heap.c > +++ b/dlls/ntdll/heap.c > @@ -116,7 +116,9 @@ C_ASSERT( sizeof(ARENA_LARGE) % LARGE_ALIGNMENT == 0 ); > /* Max size of the blocks on the free lists */ > static const SIZE_T HEAP_freeListSizes[] = > { > - 0x10, 0x20, 0x30, 0x40, 0x60, 0x80, 0x100, 0x200, 0x400, 0x1000, ~0UL > + 0x10, 0x18, 0x20, 0x28, 0x30, 0x38, 0x40, 0x48, 0x50, 0x58, 0x60, 0x68, > + 0x70, 0x78, 0x80, 0x88, 0x90, 0x98, 0xA0, 0xA8, 0xB0, 0xB8, 0xC0, 0xC8, > + 0xD0, 0xD8, 0xE0, 0E88, 0xF0, 0xF8, 0x100, 0x200, 0x400, 0x1000, ~0UL > }; > #define HEAP_NB_FREE_LISTS (sizeof(HEAP_freeListSizes)/sizeof(HEAP_freeListSizes[0])) That 0E88 looks quite wrong ;) Apart from that, although I'm not an expert for this code, this patch looks reasonable to me. Maybe we don't want so many free lists, but I don't see big downsides for that (e.g. the linear search can be replaced by a binary search, if need be). Maybe adding a smaller list at the start (e.g. 0x8) makes sense too?

Nov. 3, 2012

Re: ntdll: Fixed some heap allocation stalls

by Matej Špindler

On 03. 11. 2012 01:20, Dan Kegel wrote: > Which game were you testing? > > > League of Legends has similar issues with memory allocation. This patch does something similar: http://uz.sns.it/~ranma42/iLoL/spectator-fix-v2/0001-ntdll-Improve-performa… I have it in my tree since February and no problems so far.

Nov. 3, 2012

RE: ntdll: Fixed some heap allocation stalls

by Daniel Lehman

>> Which game were you testing? i dunno about the game, but i saw similar stalls in our software a huge batch process that consumed about 1 GB on Windows would both run slower and eat up about 2 GB on Wine before crashing i made a similar change to Steaphan's that not only improved performance, it brought the memory usage down to roughly what it was on Windows - and didn't crash i was planning on submitting it at some point if i could think of a good test to validate it. the only difference was that i added more lists, but i'd say adding more free lists definitely makes it faster daniel

Nov. 3, 2012

re: ntdll: Fixed some heap allocation stalls

by Dan Kegel

Which game were you testing?

Nov. 3, 2012

Re: [PATCH 4/4] msvcp90: Use critical sections in mutex object (try2)

by Alexandre Julliard

Piotr Caban <piotr(a)codeweavers.com> writes: > try2: > - free allocated memory in mutex_dtor > --- > dlls/msvcp100/misc.c | 18 ++++++++++++++---- > dlls/msvcp100/msvcp.h | 2 +- > dlls/msvcp60/misc.c | 16 ++++++++++++---- > dlls/msvcp71/misc.c | 18 ++++++++++++++---- > dlls/msvcp71/msvcp.h | 2 +- > dlls/msvcp90/misc.c | 18 ++++++++++++++---- > dlls/msvcp90/msvcp90.h | 2 +- > 7 files changed, 57 insertions(+), 19 deletions(-) It doesn't work here: ../../../tools/runtest -q -P wine -M msvcp90.dll -T ../../.. -p msvcp90_test.exe.so ios.c && touch ios.ok err:ntdll:RtlpWaitForCriticalSection section 0x111448 "misc.c: _Mutex critical section" wait timed out in thread 0025, blocked by 0000, retrying (60 sec) wine: Critical section 00111448 wait failed at address 0x7bc373e2 (thread 0025), starting debugger... Unhandled exception: wait failed on critical section 0x00111448 in 32-bit code (0x7bc373e2). -- Alexandre Julliard julliard(a)winehq.org

Nov. 2, 2012

re: Call for papers - FOSDEM 2013

by Dan Kegel

Francois wrote: > (*) A particularly interesting one to me is testing untrusted code on > real hardware (with possible approaches being direct access to a > graphics card from within a VM I had some fun googling your wishlist. Presumably you've seen some of these links already: http://wiki.xen.org/wiki/Xen_VGA_Passthrough http://arstechnica.com/information-technology/2012/10/nvidias-vgx-cards-bri… > or network booting (plus remote > controlled power switch?), Grub tricks). http://www.amd64.org/support/automated-test-infrastructure.html looks like it does all of the above? http://developer.amd.com/tools/opensource/AMDTapper/Pages/default.aspx A few other possibly interesting links: http://wiki.debian.org/GrubReboot http://www.passmark.com/products/rebooter.htm http://people.redhat.com/mrodrigu/slides/linuxcon-2011-autotest-lmr There are many similar pages, all somewhat buried, stale, and possibly too specific. > Another may be testing on non-VM friendly OSes, specifically Mac OS X, > though that may just be a matter of doing a more in-depth Google search > to find a legal and libvirt-compatible solution. vmware seems to work, though I haven't tried it myself yet: http://lsimons.wordpress.com/2011/06/02/libvirt-vmware-fusion-mac-os-x/ - Dan

Nov. 2, 2012

Re: [1/3] gdiplus: Add a test to check transformation order in GdipMeasureDriverString.

by Vincent Povirk

Thanks for writing tests. It turns out I was a bit confused when I asked for them. For some reason, I was was under the impression that GdipMeasureDriverString would return a measurement in device coordinates, when it should have been obvious to me that it returns world coordinates. Looking back at the existing tests confirms that, since the measurements are the same with a NULL matrix regardless of world transform. That means the test I wanted doesn't answer the question I wanted answered. I'm sorry I created extra work for you with my mistake. The approach of rotating and scaling still works, but it requires drawing text and visually inspecting the output. I've written and attached a test program that does that (with a matrix that rotates by 90 degrees and a world transform with a much greater scale on the Y axis than the X axis). It shows that things work as I expected. The text is stretched vertically AFTER the rotation is applied, and it's easy to see the difference between native's behavior and your patch. It also appears that the origin point (and presumably the later points as well) should not have the matrix applied to it. It looks to me like your patch does not transform the points in the GDI32 codepath, and thus gets the origin point correct in the testcase. It does transform them in the SOFTWARE codepath, and if I force that codepath to be taken, the text draws outside the bounds of the window. So the matrix shouldn't be passed to transform_and_round_points, and it's not necessary to add a matrix argument to that function.

Nov. 2, 2012