Bug 773903 - ~1s shutdown delay in xpcshell in xpcom shutdown. r=ted.
authorRafael Ávila de Espíndola <respindola@mozilla.com>
Mon, 23 Jul 2012 14:49:06 -0400
changeset 100185 15bb4fb1096ab12acc454a82af30bc2a6b0cc813
parent 100184 f07aca82e80e5b772f6e0def69c2ca5d08677497
child 100186 727381afb1be9f0b37944e671db22d2930b550a7
push id23169
push useremorley@mozilla.com
push dateTue, 24 Jul 2012 09:54:00 +0000
treeherdermozilla-central@a26e751bfb54 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersted
bugs773903
milestone17.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 773903 - ~1s shutdown delay in xpcshell in xpcom shutdown. r=ted. This just updates mach_override.c to a version that has a faster search for an available spot to put the branch island.
xpcom/build/mach_override.c
--- a/xpcom/build/mach_override.c
+++ b/xpcom/build/mach_override.c
@@ -1,9 +1,10 @@
-/* copied from https://github.com/rentzsch/mach_star */
+/* Copied from https://github.com/rentzsch/mach_star at revision
+ * 498e0ba31461ac6bada7fa75ce1a7009734d5a4c */
 
 /*******************************************************************************
 	mach_override.c
 		Copyright (c) 2003-2009 Jonathan 'Wolf' Rentzsch: <http://rentzsch.com>
 		Some rights reserved: <http://opensource.org/licenses/mit-license.php>
 
 	***************************************************************************/
 
@@ -20,16 +21,17 @@
 /**************************
 *	
 *	Constants
 *	
 **************************/
 #pragma mark	-
 #pragma mark	(Constants)
 
+#define kPageSize 4096
 #if defined(__ppc__) || defined(__POWERPC__)
 
 long kIslandTemplate[] = {
 	0x9001FFFC,	//	stw		r0,-4(SP)
 	0x3C00DEAD,	//	lis		r0,0xDEAD
 	0x6000BEEF,	//	ori		r0,r0,0xBEEF
 	0x7C0903A6,	//	mtctr	r0
 	0x8001FFFC,	//	lwz		r0,-4(SP)
@@ -73,44 +75,39 @@ char kIslandTemplate[] = {
 	// Now the real jump instruction
 	0xFF, 0x25, 0x00, 0x00, 0x00, 0x00,
         0x00, 0x00, 0x00, 0x00,
         0x00, 0x00, 0x00, 0x00
 };
 
 #endif
 
-#define	kAllocateHigh		1
-#define	kAllocateNormal		0
-
 /**************************
 *	
 *	Data Types
 *	
 **************************/
 #pragma mark	-
 #pragma mark	(Data Types)
 
 typedef	struct	{
 	char	instructions[sizeof(kIslandTemplate)];
-	int		allocatedHigh;
 }	BranchIsland;
 
 /**************************
 *	
 *	Funky Protos
 *	
 **************************/
 #pragma mark	-
 #pragma mark	(Funky Protos)
 
 	mach_error_t
 allocateBranchIsland(
 		BranchIsland	**island,
-		int				allocateHigh,
 		void *originalFunctionAddress);
 
 	mach_error_t
 freeBranchIsland(
 		BranchIsland	*island );
 
 #if defined(__ppc__) || defined(__POWERPC__)
 	mach_error_t
@@ -155,22 +152,20 @@ fixupInstructions(
 *	
 *******************************************************************************/
 #pragma mark	-
 #pragma mark	(Interface)
 
 #if defined(__i386__) || defined(__x86_64__)
 mach_error_t makeIslandExecutable(void *address) {
 	mach_error_t err = err_none;
-    vm_size_t pageSize;
-    host_page_size( mach_host_self(), &pageSize );
-    uintptr_t page = (uintptr_t)address & ~(uintptr_t)(pageSize-1);
+    uintptr_t page = (uintptr_t)address & ~(uintptr_t)(kPageSize-1);
     int e = err_none;
-    e |= mprotect((void *)page, pageSize, PROT_EXEC | PROT_READ | PROT_WRITE);
-    e |= msync((void *)page, pageSize, MS_INVALIDATE );
+    e |= mprotect((void *)page, kPageSize, PROT_EXEC | PROT_READ | PROT_WRITE);
+    e |= msync((void *)page, kPageSize, MS_INVALIDATE );
     if (e) {
         err = err_cannot_override;
     }
     return err;
 }
 #endif
 
     mach_error_t
@@ -238,17 +233,17 @@ mach_override_ptr(
 					(vm_address_t) originalFunctionPtr, 8, false,
 					(VM_PROT_DEFAULT | VM_PROT_COPY) );
 	}
 	if (err) fprintf(stderr, "err = %x %s:%d\n", err, __FILE__, __LINE__);
 	
 	//	Allocate and target the escape island to the overriding function.
 	BranchIsland	*escapeIsland = NULL;
 	if( !err )	
-		err = allocateBranchIsland( &escapeIsland, kAllocateHigh, originalFunctionAddress );
+		err = allocateBranchIsland( &escapeIsland, originalFunctionAddress );
 		if (err) fprintf(stderr, "err = %x %s:%d\n", err, __FILE__, __LINE__);
 
 	
 #if defined(__ppc__) || defined(__POWERPC__)
 	if( !err )
 		err = setBranchIslandTarget( escapeIsland, overrideFunctionAddress, 0 );
 	
 	//	Build the branch absolute instruction to the escape island.
@@ -280,17 +275,17 @@ mach_override_ptr(
 #endif
 	
 	//	Optionally allocate & return the reentry island. This may contain relocated
 	//  jmp instructions and so has all the same addressing reachability requirements
 	//  the escape island has to the original function, except the escape island is
 	//  technically our original function.
 	BranchIsland	*reentryIsland = NULL;
 	if( !err && originalFunctionReentryIsland ) {
-		err = allocateBranchIsland( &reentryIsland, kAllocateHigh, escapeIsland);
+		err = allocateBranchIsland( &reentryIsland, escapeIsland);
 		if( !err )
 			*originalFunctionReentryIsland = reentryIsland;
 	}
 	
 #if defined(__ppc__) || defined(__POWERPC__)	
 	//	Atomically:
 	//	o If the reentry island was allocated:
 	//		o Insert the original instruction into the reentry island.
@@ -364,117 +359,99 @@ mach_override_ptr(
 *******************************************************************************/
 #pragma mark	-
 #pragma mark	(Implementation)
 
 /***************************************************************************//**
 	Implementation: Allocates memory for a branch island.
 	
 	@param	island			<-	The allocated island.
-	@param	allocateHigh	->	Whether to allocate the island at the end of the
-								address space (for use with the branch absolute
-								instruction).
 	@result					<-	mach_error_t
 
 	***************************************************************************/
 
 	mach_error_t
 allocateBranchIsland(
 		BranchIsland	**island,
-		int				allocateHigh,
 		void *originalFunctionAddress)
 {
 	assert( island );
-	
-	mach_error_t	err = err_none;
-	
-	if( allocateHigh ) {
-		vm_size_t pageSize;
-		err = host_page_size( mach_host_self(), &pageSize );
-		if( !err ) {
-			assert( sizeof( BranchIsland ) <= pageSize );
-#if defined(__ppc__) || defined(__POWERPC__)
-			vm_address_t first = 0xfeffffff;
-			vm_address_t last = 0xfe000000 + pageSize;
-#elif defined(__x86_64__)
-			vm_address_t first = (uint64_t)originalFunctionAddress & ~(uint64_t)(((uint64_t)1 << 31) - 1) | ((uint64_t)1 << 31); // start in the middle of the page?
-			vm_address_t last = 0x0;
-#else
-			vm_address_t first = 0xffc00000;
-			vm_address_t last = 0xfffe0000;
-#endif
+	assert( sizeof( BranchIsland ) <= kPageSize );
 
-			vm_address_t page = first;
-			int allocated = 0;
-			vm_map_t task_self = mach_task_self();
-			
-			while( !err && !allocated && page != last ) {
+	vm_map_t task_self = mach_task_self();
+	vm_address_t original_address = (vm_address_t) originalFunctionAddress;
+	static vm_address_t last_allocated = 0;
+	vm_address_t address =
+		last_allocated ? last_allocated : original_address;
 
-				err = vm_allocate( task_self, &page, pageSize, 0 );
-				if( err == err_none )
-					allocated = 1;
-				else if( err == KERN_NO_SPACE ) {
-#if defined(__x86_64__)
-					page -= pageSize;
+	for (;;) {
+		vm_size_t vmsize = 0;
+		memory_object_name_t object = 0;
+		kern_return_t kr = 0;
+		vm_region_flavor_t flavor = VM_REGION_BASIC_INFO;
+		// Find the page the address is in.
+#if __WORDSIZE == 32
+		vm_region_basic_info_data_t info;
+		mach_msg_type_number_t info_count = VM_REGION_BASIC_INFO_COUNT;
+		kr = vm_region(task_self, &address, &vmsize, flavor,
+			       (vm_region_info_t)&info, &info_count, &object);
 #else
-					page += pageSize;
+		vm_region_basic_info_data_64_t info;
+		mach_msg_type_number_t info_count = VM_REGION_BASIC_INFO_COUNT_64;
+		kr = vm_region_64(task_self, &address, &vmsize, flavor,
+				  (vm_region_info_t)&info, &info_count, &object);
 #endif
-					err = err_none;
-				}
-			}
-			if( allocated )
-				*island = (BranchIsland*) page;
-			else if( !allocated && !err )
-				err = KERN_NO_SPACE;
+		if (kr != KERN_SUCCESS)
+			return kr;
+
+		// Don't underflow. This could be made to work, but this is a
+		// convenient place to give up.
+		assert((address & (kPageSize - 1)) == 0);
+		if (address == 0)
+			break;
+
+		// Go back one page.
+		vm_address_t new_address = address - kPageSize;
+#if __WORDSIZE == 64
+		if(original_address - new_address - 5 > INT32_MAX)
+			break;
+#endif
+		address = new_address;
+
+		// Try to allocate this page.
+		kr = vm_allocate(task_self, &address, kPageSize, 0);
+		if (kr == KERN_SUCCESS) {
+			*island = (BranchIsland*) address;
+			last_allocated = address;
+			return err_none;
 		}
-	} else {
-		void *block = malloc( sizeof( BranchIsland ) );
-		if( block )
-			*island = block;
-		else
-			err = KERN_NO_SPACE;
+		if (kr != KERN_NO_SPACE)
+			return kr;
 	}
-	if( !err )
-		(**island).allocatedHigh = allocateHigh;
-	
-	return err;
+
+	return KERN_NO_SPACE;
 }
 
 /***************************************************************************//**
 	Implementation: Deallocates memory for a branch island.
 	
 	@param	island	->	The island to deallocate.
 	@result			<-	mach_error_t
 
 	***************************************************************************/
 
 	mach_error_t
 freeBranchIsland(
 		BranchIsland	*island )
 {
 	assert( island );
 	assert( (*(long*)&island->instructions[0]) == kIslandTemplate[0] );
-	assert( island->allocatedHigh );
-	
-	mach_error_t	err = err_none;
-	
-	if( island->allocatedHigh ) {
-		vm_size_t pageSize;
-		err = host_page_size( mach_host_self(), &pageSize );
-		if( !err ) {
-			assert( sizeof( BranchIsland ) <= pageSize );
-			err = vm_deallocate(
-					mach_task_self(),
-					(vm_address_t) island, pageSize );
-		}
-	} else {
-		free( island );
-	}
-	
-	return err;
+	assert( sizeof( BranchIsland ) <= kPageSize );
+	return vm_deallocate( mach_task_self(), (vm_address_t) island,
+			      kPageSize );
 }
 
 /***************************************************************************//**
 	Implementation: Sets the branch island's target, with an optional
 	instruction.
 	
 	@param	island		->	The branch island to insert target into.
 	@param	branchTo	->	The address of the target.
@@ -596,16 +573,17 @@ static AsmInstructionMatch possibleInstr
 	{ 0x3, {0xFF, 0xFF, 0xFF}, {0x48, 0x89, 0xE5} },				// mov %rsp,%rbp
 	{ 0x4, {0xFF, 0xFF, 0xFF, 0x00}, {0x48, 0x83, 0xEC, 0x00} },	                // sub 0x??, %rsp
 	{ 0x4, {0xFB, 0xFF, 0x00, 0x00}, {0x48, 0x89, 0x00, 0x00} },	                // move onto rbp
 	{ 0x2, {0xFF, 0x00}, {0x41, 0x00} },						// push %rXX
 	{ 0x2, {0xFF, 0x00}, {0x85, 0x00} },						// test %rX,%rX
 	{ 0x5, {0xF8, 0x00, 0x00, 0x00, 0x00}, {0xB8, 0x00, 0x00, 0x00, 0x00} },   // mov $imm, %reg
 	{ 0x3, {0xFF, 0xFF, 0x00}, {0xFF, 0x77, 0x00} },  // pushq $imm(%rdi)
 	{ 0x2, {0xFF, 0xFF}, {0x31, 0xC0} },						// xor %eax, %eax
+    { 0x2, {0xFF, 0xFF}, {0x89, 0xF8} },			// mov %edi, %eax
 	{ 0x0 }
 };
 #endif
 
 static Boolean codeMatchesInstruction(unsigned char *code, AsmInstructionMatch* instruction) 
 {
 	Boolean match = true;