Re: [PATCH 5/7] server: Store and return security attributes with extended file attributes.
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=22707 Your paranoid android. === WNT4WSSP6 (32 bit directory) === Timeout
Re: [PATCH 6/7] ntdll: Inherit security attributes from parent directories.
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=22708 Your paranoid android. === WNT4WSSP6 (32 bit directory) === Timeout
Re: [PATCH 5/7] server: Store and return security attributes with extended file attributes.
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=22706 Your paranoid android. === WNT4WSSP6 (32 bit security) === Timeout === W2KPROSP4 (32 bit security) === security.c:4411: Test failed: GetAclInformation returned unexpected entry count (5 != 2). security.c:4429: Test failed: Administators Group ACE != Administators Group SID. security.c:4432: Test failed: Administators Group ACE has unexpected mask (0x1000 != 0x1f01ff) === WXPPROSP3 (32 bit security) === security.c:4411: Test failed: GetAclInformation returned unexpected entry count (6 != 2). === W2K3R2SESP2 (32 bit security) === security.c:4411: Test failed: GetAclInformation returned unexpected entry count (5 != 2). === WVISTAADM (32 bit security) === security.c:4411: Test failed: GetAclInformation returned unexpected entry count (6 != 2). === W2K8SE (32 bit security) === security.c:4411: Test failed: GetAclInformation returned unexpected entry count (5 != 2). === W7PRO (32 bit security) === security.c:4411: Test failed: GetAclInformation returned unexpected entry count (6 != 2). === W7PROX64 (32 bit security) === security.c:4411: Test failed: GetAclInformation returned unexpected entry count (6 != 2). === TEST64_W7SP1 (32 bit security) === security.c:4411: Test failed: GetAclInformation returned unexpected entry count (6 != 2). === W7PROX64 (64 bit security) === security.c:4411: Test failed: GetAclInformation returned unexpected entry count (6 != 2). === TEST64_W7SP1 (64 bit security) === security.c:4411: Test failed: GetAclInformation returned unexpected entry count (6 != 2).
Re: [PATCH 3/7] advapi: Implement GetNamedSecurityInfoW on top of GetSecurityInfo.
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=22704 Your paranoid android. === WNT4WSSP6 (32 bit security) === Timeout === W2KPROSP4 (32 bit security) === security.c:4411: Test failed: GetAclInformation returned unexpected entry count (5 != 2). security.c:4429: Test failed: Administators Group ACE != Administators Group SID. security.c:4432: Test failed: Administators Group ACE has unexpected mask (0x1000 != 0x1f01ff) === WXPPROSP3 (32 bit security) === security.c:4411: Test failed: GetAclInformation returned unexpected entry count (6 != 2). === W2K3R2SESP2 (32 bit security) === security.c:4411: Test failed: GetAclInformation returned unexpected entry count (5 != 2). === WVISTAADM (32 bit security) === security.c:4411: Test failed: GetAclInformation returned unexpected entry count (6 != 2). === W2K8SE (32 bit security) === security.c:4411: Test failed: GetAclInformation returned unexpected entry count (5 != 2). === W7PRO (32 bit security) === security.c:4411: Test failed: GetAclInformation returned unexpected entry count (6 != 2). === W7PROX64 (32 bit security) === security.c:4411: Test failed: GetAclInformation returned unexpected entry count (6 != 2). === TEST64_W7SP1 (32 bit security) === security.c:4411: Test failed: GetAclInformation returned unexpected entry count (6 != 2). === W7PROX64 (64 bit security) === security.c:4411: Test failed: GetAclInformation returned unexpected entry count (6 != 2). === TEST64_W7SP1 (64 bit security) === security.c:4411: Test failed: GetAclInformation returned unexpected entry count (6 != 2).
Re: [PATCH 4/7] server: Create directories with the specified security attributes.
Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=22705 Your paranoid android. === WNT4WSSP6 (32 bit directory) === Timeout
testsuite on windows 8
Am 28.10.2012 13:50, schrieb André Hentschel: > Am 28.10.2012 09:06, schrieb Gerold Jens Wucherpfennig: >> (I just bought Windows 8, >> so in return I want to spend some time >> and money to wine...) >> > > One easy thing you could do is to run the testsuite on win8 (see the bottom > of http://test.winehq.org/). running the testsuite on Windows 8 x64 german produced 85 errors. Because of this pile of errors the testsuite refused to submit the test data... Best Regards, Gerold > >> fixme:vbscript:parse_script parser failed on parsing L"1\r\nConst >> msiInstallStateAdvertise = 1\r\nConst msiInstallStateAbsent = 2\r\nConst >> msiInstallStateLocal = 3\r\nConst msiInstallStateSource = 4\r\nConst >> msiInstallStateDefault = 5\r\nDim nRemoveVBAEN:nRemoveVBAEN = 0\r\nDim >> nRemoveVBACZ:nRemoveVBACZ = 0\r\nDim nRemoveVBANL:nRemoveVBAPL = >> 0\r\n\r\nOn "... > > As vbscript knows about "Const" i'd say the parser starts with parsing a > single "1" and fails. > Not sure if that's the biggest problem for the installer, as i see much ERRs > in the output. > >
Re: testsuite on windows 8
Am 03.11.2012 19:12, schrieb Gerold Jens Wucherpfennig: > Am 28.10.2012 13:50, schrieb André Hentschel: >> Am 28.10.2012 09:06, schrieb Gerold Jens Wucherpfennig: >>> (I just bought Windows 8, >>> so in return I want to spend some time >>> and money to wine...) >>> >> >> One easy thing you could do is to run the testsuite on win8 (see the bottom >> of http://test.winehq.org/). > > running the testsuite on Windows 8 x64 german > produced 85 errors. Because of this pile of errors > the testsuite refused to submit the test data... Yes, that's the actual state of testing win8. There are two workarounds: 1) run winetest.exe search c:\users\username\appdata\local\temp by date for some res*.tmp file copy it into the same folder as winetest.exe and run winetest.exe -s res.tmp 2) run winetest.exe -o heute.test to not save the log into a temp file run winetest.exe -s heute.test to send the just created file You can try 1) to see if your last temp file is still there and send it. -- Best Regards, André Hentschel
Re: ntdll: Fixed some heap allocation stalls
On 11/03/2012 10:28 AM, Matteo Bruni wrote:
2012/11/3 Steaphan Greene:
On 11/03/2012 09:04 AM, Matteo Bruni wrote:
2012/11/2 Steaphan Greene:
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.
Done.
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.
That might be fine, though I haven't found any evidence that this number
of bins has any significant downside other than one pointer per bin per
heap, which seems tiny for the gains it produces. It does search them
linearly, which, as you mention, is easy to fix with a binary search,
but this number is still so small, it's really not that significant.
With a larger number of bins, it might be more of an issue, but if it
were also setup with a regular pattern of bins, then finding the right
bin could be a simple O(1) calculation, and even faster than a binary
search.
I might be curious enough right now to implement that whole thing
myself, if it's desired.
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.
Well, my code instrumentation consisted of fprintf()s, which I
redirected through grep, sort, and uniq. So, I don't have anything
sophisticated to share on that end.
--
Steaphan Greene
Re: ntdll: Fixed some heap allocation stalls
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-performace-of-heap-allocation-v2.patch 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
Re: ntdll: Fixed some heap allocation stalls
2012/11/3 Steaphan Greene :
> On 11/03/2012 09:04 AM, Matteo Bruni wrote:
>>
>> 2012/11/2 Steaphan Greene:
>>>
>>> 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.
Re: ntdll: Fixed some heap allocation stalls
On 11/03/2012 09:04 AM, Matteo Bruni wrote:
2012/11/2 Steaphan Greene:
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
Re: ntdll: Fixed some heap allocation stalls
2012/11/2 Steaphan Greene :
> 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?
Re: ntdll: Fixed some heap allocation stalls
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-performace-of-heap-allocation-v2.patch I have it in my tree since February and no problems so far.
