Re: [PATCH] shadow: Optimize shadowUpdatePacked(). (#26973)
On Sat, Sep 11, 2010 at 5:55 PM, Matt Turner matts...@gmail.com wrote: From: Adam Jackson a...@redhat.com Signed-off-by: Matt Turner matts...@gmail.com --- I was bug triaging and came across 26973 and remembered seeing it on the ml at some point recently. Here's a patch, as suggested by ajax [1]. Corbin: are there any patches from your bug triage branch that need to go into the xserver? [1] http://lists.freedesktop.org/archives/xorg-devel/2010-March/00.html miext/shadow/shpacked.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/miext/shadow/shpacked.c b/miext/shadow/shpacked.c index 20d2ea1..06606bc 100644 --- a/miext/shadow/shpacked.c +++ b/miext/shadow/shpacked.c @@ -102,8 +102,8 @@ shadowUpdatePacked (ScreenPtr pScreen, width -= i; scr += i; #define PickBit(a,i) (((a) (i)) 1) - while (i--) - *win++ = *sha++; + memcpy(win, sha, i * sizeof(FbBits)); + sha += i; } shaLine += shaStride; y++; -- 1.7.1 Hi Keith, Can you vacuum this one up? Soren was kind enough to review it. And since I'm sending it, have a Reviewed-by: Matt Turner matts...@gmail.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] shadow: Optimize shadowUpdatePacked(). (#26973)
Matt Turner matts...@gmail.com writes: So, do we want this patch? It seems like it does deobfuscate the code a bit, and memcpy is going to be more efficient than byte-by-byte copies. Matt Can I get some Reviewed-bys or Acked-bys? Reviewed-by: Soren Sandmann sandm...@daimi.au.dk The PickBit macro seems to be unused, but I guess that's independent of this patch. Soren ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] shadow: Optimize shadowUpdatePacked(). (#26973)
On Tue, Sep 21, 2010 at 10:29 AM, Matt Turner matts...@gmail.com wrote: On Sat, Sep 11, 2010 at 5:55 PM, Matt Turner matts...@gmail.com wrote: From: Adam Jackson a...@redhat.com Signed-off-by: Matt Turner matts...@gmail.com --- I was bug triaging and came across 26973 and remembered seeing it on the ml at some point recently. Here's a patch, as suggested by ajax [1]. Corbin: are there any patches from your bug triage branch that need to go into the xserver? [1] http://lists.freedesktop.org/archives/xorg-devel/2010-March/00.html miext/shadow/shpacked.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/miext/shadow/shpacked.c b/miext/shadow/shpacked.c index 20d2ea1..06606bc 100644 --- a/miext/shadow/shpacked.c +++ b/miext/shadow/shpacked.c @@ -102,8 +102,8 @@ shadowUpdatePacked (ScreenPtr pScreen, width -= i; scr += i; #define PickBit(a,i) (((a) (i)) 1) - while (i--) - *win++ = *sha++; + memcpy(win, sha, i * sizeof(FbBits)); + sha += i; } shaLine += shaStride; y++; -- 1.7.1 So, do we want this patch? It seems like it does deobfuscate the code a bit, and memcpy is going to be more efficient than byte-by-byte copies. Matt Can I get some Reviewed-bys or Acked-bys? ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] shadow: Optimize shadowUpdatePacked(). (#26973)
On Tue, 2010-09-21 at 10:29 -0400, Matt Turner wrote: On Sat, Sep 11, 2010 at 5:55 PM, Matt Turner matts...@gmail.com wrote: diff --git a/miext/shadow/shpacked.c b/miext/shadow/shpacked.c index 20d2ea1..06606bc 100644 --- a/miext/shadow/shpacked.c +++ b/miext/shadow/shpacked.c @@ -102,8 +102,8 @@ shadowUpdatePacked (ScreenPtr pScreen, width -= i; scr += i; #define PickBit(a,i) (((a) (i)) 1) - while (i--) - *win++ = *sha++; + memcpy(win, sha, i * sizeof(FbBits)); + sha += i; } shaLine += shaStride; y++; -- 1.7.1 So, do we want this patch? It seems like it does deobfuscate the code a bit, and memcpy is going to be more efficient than byte-by-byte copies. I think this is okay as long as we have the invariant that the shadow and the shadowed are not allowed to have aliased storage. Which seems entirely reasonable, so, yeah, we should take this. In related news, we never fixed this: http://lists.freedesktop.org/archives/xorg/2010-February/049026.html - ajax signature.asc Description: This is a digitally signed message part ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] shadow: Optimize shadowUpdatePacked(). (#26973)
On Wed, 2010-09-22 at 10:19 -0400, Adam Jackson wrote: In related news, we never fixed this: http://lists.freedesktop.org/archives/xorg/2010-February/049026.html IIRC, the software path for CopyArea is pretty poor and makes things very slow if you scroll in a nontrivial direction on an unaccelerated server. However it does seem to avoid that bug by detecting the nontrivial cases and using a temporary scanline buffer. The right solution is probably to implement an overlap-capable CopyArea routine in Pixman and have CopyArea call it when possible. -- -- From: Jonathan Morton jonathan.mor...@movial.com ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] shadow: Optimize shadowUpdatePacked(). (#26973)
On Sat, Sep 11, 2010 at 5:55 PM, Matt Turner matts...@gmail.com wrote: From: Adam Jackson a...@redhat.com Signed-off-by: Matt Turner matts...@gmail.com --- I was bug triaging and came across 26973 and remembered seeing it on the ml at some point recently. Here's a patch, as suggested by ajax [1]. Corbin: are there any patches from your bug triage branch that need to go into the xserver? [1] http://lists.freedesktop.org/archives/xorg-devel/2010-March/00.html miext/shadow/shpacked.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/miext/shadow/shpacked.c b/miext/shadow/shpacked.c index 20d2ea1..06606bc 100644 --- a/miext/shadow/shpacked.c +++ b/miext/shadow/shpacked.c @@ -102,8 +102,8 @@ shadowUpdatePacked (ScreenPtr pScreen, width -= i; scr += i; #define PickBit(a,i) (((a) (i)) 1) - while (i--) - *win++ = *sha++; + memcpy(win, sha, i * sizeof(FbBits)); + sha += i; } shaLine += shaStride; y++; -- 1.7.1 So, do we want this patch? It seems like it does deobfuscate the code a bit, and memcpy is going to be more efficient than byte-by-byte copies. Matt ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH] shadow: Optimize shadowUpdatePacked(). (#26973)
From: Adam Jackson a...@redhat.com Signed-off-by: Matt Turner matts...@gmail.com --- I was bug triaging and came across 26973 and remembered seeing it on the ml at some point recently. Here's a patch, as suggested by ajax [1]. Corbin: are there any patches from your bug triage branch that need to go into the xserver? [1] http://lists.freedesktop.org/archives/xorg-devel/2010-March/00.html miext/shadow/shpacked.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/miext/shadow/shpacked.c b/miext/shadow/shpacked.c index 20d2ea1..06606bc 100644 --- a/miext/shadow/shpacked.c +++ b/miext/shadow/shpacked.c @@ -102,8 +102,8 @@ shadowUpdatePacked (ScreenPtr pScreen, width -= i; scr += i; #define PickBit(a,i) (((a) (i)) 1) - while (i--) - *win++ = *sha++; + memcpy(win, sha, i * sizeof(FbBits)); + sha += i; } shaLine += shaStride; y++; -- 1.7.1 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
[PATCH] shadow: Optimize shadowUpdatePacked(). (#26973)
From: Ilpo Ruotsalainen ilpo.ruotsalai...@movial.fi v2: Also remove the very very last reference to PickBits, and handle some pointer maths. Signed-off-by: Corbin Simpson mostawesomed...@gmail.com Reviewed-by: Matt Turner matts...@gmail.com --- miext/shadow/shpacked.c |5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/miext/shadow/shpacked.c b/miext/shadow/shpacked.c index 6736162..6339a80 100644 --- a/miext/shadow/shpacked.c +++ b/miext/shadow/shpacked.c @@ -101,9 +101,8 @@ shadowUpdatePacked (ScreenPtr pScreen, i = width; width -= i; scr += i; -#define PickBit(a,i) (((a) (i)) 1) - while (i--) - *win++ = *sha++; + memcpy(win, sha, i * sizeof(FbBits)); + sha += i; } shaLine += shaStride; y++; -- 1.6.6.1 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
