To start off with, we're sorry for any confusion we caused, but this
issue does not have any interaction with the heap-walking code. The
problem definitely has to do with the readscreen function. Making
readscreen() a no-op decreases initialization time by 3 seconds.

Delving more into the issue, we added debug prints to measure the
timing [1] of various sections of readscreen(). For our particular
case, the inner loop is called 47 times. Inside the loop, each
invocation of GetBitmapBits takes about 78ms. The total runtime of
readscreen alone was 3640ms.

The MSDN page indicates that GetBitmapBits is a 16-bit Windows
compatibility function [2]. Some random web searches seem to indicate
that it has to go through multiple layers of compatibility code,
involving acquiring a global lock of some kind.

The same MSDN page points to the suggested replacement function,
GetDIBits [3].

Please find attached a patch to modify readscreen to use the newer
GetDIBits function. On our problem machine, this decreases the time
taken in readscreen() to 94ms. The majority of the code in the patch
comes from a MSDN example on device contexts [4].

To verify that the new code functions identically, we modified the
original code and printed out the hex of the hashed data. For each
chunk of framebuffer, the hash is
1adc95bebe9eea8c112d40cd4ab7a8d75c4f961, in both the sets of code.

Of special note is that this code runs in the context of the anonymous
IIS user, which is unlikely to have a useful screen. One might wonder
how often this happens. Presumably, user-mode software can actually
access the screen, but server-type software will not have a screen.

Please let me know if we can provide further data or help. Any
comments, insight, or suggestions on the patch are appreciated.

Thanks!

[1] The timing information was provided by the Windows function
    GetSystemTimeAsFileTime.
[2] http://msdn.microsoft.com/en-us/library/dd144850(VS.85).aspx
[3] http://msdn.microsoft.com/en-us/library/dd144879(v=VS.85).aspx
[4] http://msdn.microsoft.com/en-us/library/dd183402(v=VS.85).aspx

Jake Goulding | Software Engineer
gould...@vivisimo.com | Connect: www.vivisimo.com
Vivisimo - Information Optimized

----- "Brian Makin" <ma...@vivisimo.com> wrote:

> This is Windows 2003, 64 bit, and it's definitely in RAND_screen.
> I'm trying to move things to 1.0.0a now.
commit 3e66cd55c48596fb42da9c98635a347a5159eec2
Author: Jake Goulding <gould...@vivisimo.com>
Date:   Fri Jul 2 14:42:32 2010 -0400

    Update Windows function that gets the bits of the screen

diff --git a/source/openssl-0.9.8l/crypto/rand/rand_win.c b/source/openssl-0.9.8l/crypto/rand/rand_win.c
index 00dbe42..11394af 100644
--- a/source/openssl-0.9.8l/crypto/rand/rand_win.c
+++ b/source/openssl-0.9.8l/crypto/rand/rand_win.c
@@ -690,9 +690,7 @@ static void readscreen(void)
 {
 #if !defined(OPENSSL_SYS_WINCE) && !defined(OPENSSL_SYS_WIN32_CYGWIN)
   HDC		hScrDC;		/* screen DC */
-  HDC		hMemDC;		/* memory DC */
   HBITMAP	hBitmap;	/* handle for our bitmap */
-  HBITMAP	hOldBitmap;	/* handle for previous bitmap */
   BITMAP	bm;		/* bitmap properties */
   unsigned int	size;		/* size of bitmap */
   char		*bmbits;	/* contents of bitmap */
@@ -700,13 +698,13 @@ static void readscreen(void)
   int		h;		/* screen height */
   int		y;		/* y-coordinate of screen lines to grab */
   int		n = 16;		/* number of screen lines to grab at a time */
+  BITMAPINFOHEADER   bi;	/* info about the bitmap  */
 
   if (GetVersion() >= 0x80000000 || !OPENSSL_isservice())
     return;
 
-  /* Create a screen DC and a memory DC compatible to screen DC */
-  hScrDC = CreateDC(TEXT("DISPLAY"), NULL, NULL, NULL);
-  hMemDC = CreateCompatibleDC(hScrDC);
+  /* Get a reference to the screen DC */
+  hScrDC = GetDC(NULL);
 
   /* Get screen resolution */
   w = GetDeviceCaps(hScrDC, HORZRES);
@@ -715,13 +713,22 @@ static void readscreen(void)
   /* Create a bitmap compatible with the screen DC */
   hBitmap = CreateCompatibleBitmap(hScrDC, w, n);
 
-  /* Select new bitmap into memory DC */
-  hOldBitmap = SelectObject(hMemDC, hBitmap);
-
   /* Get bitmap properties */
   GetObject(hBitmap, sizeof(BITMAP), (LPSTR)&bm);
   size = (unsigned int)bm.bmWidthBytes * bm.bmHeight * bm.bmPlanes;
 
+  bi.biSize = sizeof(BITMAPINFOHEADER);
+  bi.biWidth = bm.bmWidth;
+  bi.biHeight = bm.bmHeight;
+  bi.biPlanes = bm.bmPlanes;
+  bi.biBitCount = bm.bmBitsPixel;
+  bi.biCompression = BI_RGB;
+  bi.biSizeImage = 0;
+  bi.biXPelsPerMeter = 0;
+  bi.biYPelsPerMeter = 0;
+  bi.biClrUsed = 0;
+  bi.biClrImportant = 0;
+
   bmbits = OPENSSL_malloc(size);
   if (bmbits) {
     /* Now go through the whole screen, repeatedly grabbing n lines */
@@ -729,11 +736,9 @@ static void readscreen(void)
     	{
 	unsigned char md[MD_DIGEST_LENGTH];
 
-	/* Bitblt screen DC to memory DC */
-	BitBlt(hMemDC, 0, 0, w, n, hScrDC, 0, y, SRCCOPY);
-
-	/* Copy bitmap bits from memory DC to bmbits */
-	GetBitmapBits(hBitmap, size, bmbits);
+	/* Copy the bits of the current line range into the buffer */
+	GetDIBits(hScrDC, hBitmap, y, n,
+		  bmbits, (BITMAPINFO *)&bi, DIB_RGB_COLORS);
 
 	/* Get the hash of the bitmap */
 	MD(bmbits,size,md);
@@ -745,13 +750,9 @@ static void readscreen(void)
     OPENSSL_free(bmbits);
   }
 
-  /* Select old bitmap back into memory DC */
-  hBitmap = SelectObject(hMemDC, hOldBitmap);
-
   /* Clean up */
   DeleteObject(hBitmap);
-  DeleteDC(hMemDC);
-  DeleteDC(hScrDC);
+  ReleaseDC(NULL, hScrDC);
 #endif /* !OPENSSL_SYS_WINCE */
 }
 

Reply via email to