[PATCH] suggestion for faster pseudo-reloc.
Jin-woo Ye
jojelino@gmail.com
Sun Aug 26 01:57:00 GMT 2012
This patch fixes the problem making pseudo-reloc too slow when there is
many pseudo-reloc entries in rdata section by deciding when not to call
Virtual{Query,Protect} to save overhead.
I tested this patch and time taken for pseudo-reloc reduced 1800ms to
16ms for 3682 entries.
Please review this patch.
--
Regards.
-------------- next part --------------
2012-08-26 <jojelino@gmail.com>
* pseudo-reloc.cc (auto_protect_for): Define.
(__write_memory): Use auto_protect_for, add verbose message displaying the number of items.
(_pei386_runtime_relocator):Add verbose message displaying time taken for pseudo-reloc.
Index: winsup/cygwin/pseudo-reloc.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/pseudo-reloc.cc,v
retrieving revision 1.10
diff -u -p -r1.10 pseudo-reloc.cc
--- winsup/cygwin/pseudo-reloc.cc 16 Aug 2012 23:34:44 -0000 1.10
+++ winsup/cygwin/pseudo-reloc.cc 26 Aug 2012 01:47:13 -0000
@@ -125,7 +125,78 @@ __report_error (const char *msg, ...)
abort ();
#endif
}
-
+/**
+ * This function automatically sets addr as PAGE_EXECUTE_READWRITE
+ * by deciding whether VirtualQuery for the addr is actually needed.
+ * and it assumes that it is called in LdrpCallInitRoutine.
+ * hence not thread safe.
+ */
+static void
+auto_protect_for(void* addr)
+{
+ static MEMORY_BASIC_INFORMATION c[2];
+ static int current = 0;
+ static int state = 0;
+ static DWORD oldprot;
+ if (addr == 0)
+ state = 2;
+ if (state == 0)
+ {
+ state = 1;
+#if 0
+ debug_printf("mark as initial %p", addr);
+#endif
+ loop:
+ {
+ MEMORY_BASIC_INFORMATION * const b = &c[current];
+ if (!VirtualQuery(addr, b, sizeof(MEMORY_BASIC_INFORMATION)))
+ {
+ __report_error(" VirtualQuery failed for %d bytes at address %p",
+ (int) sizeof(MEMORY_BASIC_INFORMATION), addr);
+ }
+ /* Temporarily allow write access to read-only protected memory. */
+ if (b->Protect != PAGE_EXECUTE_READWRITE
+ && b->Protect != PAGE_READWRITE)
+ VirtualProtect(b->BaseAddress, b->RegionSize,
+ PAGE_EXECUTE_READWRITE, &oldprot);
+ }
+ }
+ else if (state == 1)
+ {
+ MEMORY_BASIC_INFORMATION * const b = &c[current];
+ void* ptr = ((void*) ((ptrdiff_t) b->BaseAddress
+ + (ptrdiff_t) b->RegionSize));
+#if 0
+ debug_printf("reloc base %p bound %p", b->BaseAddress,ptr);
+#endif
+ if ((addr >= b->BaseAddress) && addr < ptr)
+ {
+#if 0
+ debug_printf("mark as old %p", addr);
+#endif
+ }
+ else
+ {
+#if 0
+ debug_printf("mark as new %p", addr);
+#endif
+ /* Restore original protection. */
+ if (b->Protect != PAGE_EXECUTE_READWRITE
+ && b->Protect != PAGE_READWRITE)
+ VirtualProtect(b->BaseAddress, b->RegionSize, oldprot, &oldprot);
+ current = (current + 1) % 2;
+ goto loop;
+ }
+ }
+ else if (state == 2)
+ {
+ MEMORY_BASIC_INFORMATION * const b = &c[current];
+ if (b->Protect != PAGE_EXECUTE_READWRITE && b->Protect != PAGE_READWRITE)
+ VirtualProtect(b->BaseAddress, b->RegionSize, oldprot, &oldprot);
+ current = 0;
+ state = 0;
+ }
+}
/* This function temporarily marks the page containing addr
* writable, before copying len bytes from *src to *addr, and
* then restores the original protection settings to the page.
@@ -142,27 +213,14 @@ __report_error (const char *msg, ...)
static void
__write_memory (void *addr, const void *src, size_t len)
{
- MEMORY_BASIC_INFORMATION b;
- DWORD oldprot;
-
if (!len)
return;
-
- if (!VirtualQuery (addr, &b, sizeof (b)))
- {
- __report_error (" VirtualQuery failed for %d bytes at address %p",
- (int) sizeof (b), addr);
- }
-
- /* Temporarily allow write access to read-only protected memory. */
- if (b.Protect != PAGE_EXECUTE_READWRITE && b.Protect != PAGE_READWRITE)
- VirtualProtect (b.BaseAddress, b.RegionSize, PAGE_EXECUTE_READWRITE,
- &oldprot);
+ /*
+ * Hopefully it would eliminate unnecessary call to Virtual{Query,Protect}.
+ */
+ auto_protect_for(addr);
/* write the data. */
memcpy (addr, src, len);
- /* Restore original protection. */
- if (b.Protect != PAGE_EXECUTE_READWRITE && b.Protect != PAGE_READWRITE)
- VirtualProtect (b.BaseAddress, b.RegionSize, oldprot, &oldprot);
}
#define RP_VERSION_V1 0
@@ -222,8 +280,10 @@ do_pseudo_reloc (void * start, void * en
/*************************
* Handle v1 relocations *
*************************/
- runtime_pseudo_reloc_item_v1 * o;
- for (o = (runtime_pseudo_reloc_item_v1 *) v2_hdr;
+ runtime_pseudo_reloc_item_v1 * o=(runtime_pseudo_reloc_item_v1*)v2_hdr;
+ debug_printf("Iterating through %d items",
+ ((ptrdiff_t)end-(ptrdiff_t)o)/sizeof(runtime_pseudo_reloc_item_v1));
+ for (;
o < (runtime_pseudo_reloc_item_v1 *)end;
o++)
{
@@ -232,6 +292,10 @@ do_pseudo_reloc (void * start, void * en
newval = (*((DWORD*) reloc_target)) + o->addend;
__write_memory ((void *) reloc_target, &newval, sizeof (DWORD));
}
+ /*
+ * Initialize variables in function for latter use.
+ */
+ auto_protect_for(0);
return;
}
@@ -251,7 +315,8 @@ do_pseudo_reloc (void * start, void * en
/* Walk over header. */
r = (runtime_pseudo_reloc_item_v2 *) &v2_hdr[1];
-
+ debug_printf("Iterating through %d items",
+ ((ptrdiff_t)end-(ptrdiff_t)r)/sizeof(runtime_pseudo_reloc_item_v2));
for (; r < (runtime_pseudo_reloc_item_v2 *) end; r++)
{
/* location where new address will be written */
@@ -322,7 +387,11 @@ do_pseudo_reloc (void * start, void * en
break;
#endif
}
- }
+ }
+ /*
+ * Initialize variables in function for latter use.
+ */
+ auto_protect_for(0);
}
#ifdef __CYGWIN__
@@ -330,7 +399,12 @@ extern "C" void
_pei386_runtime_relocator (per_process *u)
{
if (u && CYGWIN_VERSION_USE_PSEUDO_RELOC_IN_DLL (u))
- do_pseudo_reloc (u->pseudo_reloc_start, u->pseudo_reloc_end, u->image_base);
+ {
+ DWORD tick = GetTickCount();
+ do_pseudo_reloc(u->pseudo_reloc_start, u->pseudo_reloc_end, u->image_base);
+ debug_printf("HMODULE %p did pseudo-relocation in %Dms",
+ u->image_base, GetTickCount()-tick);
+ }
}
#else
extern "C" void
More information about the Cygwin-patches
mailing list