Last Comment Bug 478044 - Enable jemalloc for Windows Mobile builds
: Enable jemalloc for Windows Mobile builds
Status: RESOLVED FIXED
: fixed1.9.1
Product: Core
Classification: Components
Component: Memory Allocator (show other bugs)
: Trunk
: ARM Windows Mobile 6 Professional
: -- normal with 1 vote (vote)
: ---
Assigned To: Brad Lassey [:blassey] (use needinfo?)
:
Mentors:
: 477956 (view as bug list)
Depends on: 476201 481579 481582 481584 481585 481586
Blocks: 427107 477956
  Show dependency treegraph
 
Reported: 2009-02-11 11:14 PST by Mark Finkle (:mfinkle) (use needinfo?)
Modified: 2009-04-16 07:53 PDT (History)
15 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
1.0a1-wm+


Attachments
WIP patch w/debugging cruft (48.00 KB, patch)
2009-02-24 17:10 PST, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Splinter Review
WIP v.2 (91.85 KB, patch)
2009-02-25 21:01 PST, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Splinter Review
WIP v.3 (95.39 KB, patch)
2009-02-26 13:26 PST, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Splinter Review
WIP v.4 (92.95 KB, patch)
2009-02-27 09:43 PST, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Splinter Review
cleaned up patch (87.93 KB, patch)
2009-02-28 10:54 PST, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Splinter Review
cleaned up patch v.2 (71.91 KB, patch)
2009-02-28 12:52 PST, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Splinter Review
cleaned up patch v.3 (75.22 KB, patch)
2009-03-03 03:50 PST, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Splinter Review
just jemalloc changes (37.10 KB, patch)
2009-03-03 18:28 PST, Brad Lassey [:blassey] (use needinfo?)
pavlov: review-
Details | Diff | Splinter Review
patch v.2 (19.23 KB, patch)
2009-03-17 13:03 PDT, Brad Lassey [:blassey] (use needinfo?)
pavlov: review+
blassey.bugs: review+
mbeltzner: approval1.9.1-
Details | Diff | Splinter Review
extra assertion (349 bytes, patch)
2009-03-24 14:13 PDT, Brad Lassey [:blassey] (use needinfo?)
pavlov: review+
Details | Diff | Splinter Review
if ret != requested address, free ret and return null (733 bytes, patch)
2009-03-25 11:21 PDT, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Splinter Review
reserves, but doesn't commit unaligned addersses (4.23 KB, patch)
2009-03-30 02:17 PDT, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Splinter Review
patch v.4 (4.91 KB, patch)
2009-03-30 14:27 PDT, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Splinter Review
patch v.5 (5.71 KB, patch)
2009-03-30 20:57 PDT, Brad Lassey [:blassey] (use needinfo?)
no flags Details | Diff | Splinter Review
patch v.6 (7.81 KB, patch)
2009-03-30 21:31 PDT, Brad Lassey [:blassey] (use needinfo?)
pavlov: review+
jasone: review+
mbeltzner: approval1.9.1-
Details | Diff | Splinter Review
A patch (669 bytes, patch)
2009-04-01 16:23 PDT, Hiroyuki Ikezoe (:hiro)
blassey.bugs: review-
Details | Diff | Splinter Review
rolled up patch (15.05 KB, patch)
2009-04-15 12:20 PDT, Brad Lassey [:blassey] (use needinfo?)
mbeltzner: approval1.9.1+
Details | Diff | Splinter Review
[checked in] add to mozconfig (840 bytes, patch)
2009-04-15 19:23 PDT, Aki Sasaki [:aki]
blassey.bugs: review+
Details | Diff | Splinter Review

Description Mark Finkle (:mfinkle) (use needinfo?) 2009-02-11 11:14:54 PST
Windows Mobile builds are using VS9 ARM compiler and the Windows Mobile 6.1 SDK
Comment 1 Brad Lassey [:blassey] (use needinfo?) 2009-02-24 17:10:42 PST
Created attachment 364002 [details] [diff] [review]
WIP patch w/debugging cruft

this allows us to build and run the "alloc_toolkit" tests.  When building Fennec for windows ce the build fails when linking xr-stub because free is defined twice (jemalloc and coredll)
Comment 2 Brad Lassey [:blassey] (use needinfo?) 2009-02-25 21:01:04 PST
Created attachment 364252 [details] [diff] [review]
WIP v.2

this allows us to build, link and "run".  Fennec crashes when freeing memory pointing to "\bin\xulrunner", which is a converted commandline argument.
Comment 3 Brad Lassey [:blassey] (use needinfo?) 2009-02-26 13:26:14 PST
Created attachment 364378 [details] [diff] [review]
WIP v.3

this gets further.  I think there is something wrong the implementation of operator new.
Comment 4 Brad Lassey [:blassey] (use needinfo?) 2009-02-27 09:43:50 PST
Created attachment 364536 [details] [diff] [review]
WIP v.4

This patch works.  I can load planet.m.o with a debug, non-opt build.  It needs a lot of clean up, but if stuart and vlad could take a look for places I've gone off in the wrong direction, I'd appreciate it.
Comment 5 Brad Lassey [:blassey] (use needinfo?) 2009-02-28 10:54:36 PST
Created attachment 364699 [details] [diff] [review]
cleaned up patch
Comment 6 Brad Lassey [:blassey] (use needinfo?) 2009-02-28 10:56:46 PST
This patch applies over the patch on bug 476201.  Since I see no way to build the shunt conditional on MOZ_MEMORY other than by getting rid of the vc project, I'm marking this as dependent on it.
Comment 7 Brad Lassey [:blassey] (use needinfo?) 2009-02-28 12:52:46 PST
Created attachment 364707 [details] [diff] [review]
cleaned up patch v.2
Comment 8 Brad Lassey [:blassey] (use needinfo?) 2009-02-28 14:51:55 PST
I've run this through the try server a few times.  The win32 builder fails, but I can't find an error in the log other than makefile.sub not being found (which is also in the logs of successful builds). I don't have visual studio 2005, so I can't reproduce locally.  Any suggestions/help would be appreciated.

http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1235854368.1235859298.25814.gz&fulltext=1
Comment 9 Brad Lassey [:blassey] (use needinfo?) 2009-03-03 03:50:55 PST
Created attachment 365165 [details] [diff] [review]
cleaned up patch v.3

fixed chunk_alloc_map to loop if unsuccessful allocating memory
Comment 10 Brad Lassey [:blassey] (use needinfo?) 2009-03-03 18:28:17 PST
Created attachment 365355 [details] [diff] [review]
just jemalloc changes
Comment 11 Stuart Parmenter 2009-03-11 12:54:52 PDT
Comment on attachment 365355 [details] [diff] [review]
just jemalloc changes

there are a lot of random ifdef WINCE that are out of place, and should be MOZ_MEMORY_WINCE.  

I suggest that you implement a getenv function that returns null rather than ifdefing it out down below

for CHUNK_2POW_DEFAULT	, it might be better to use malloc_options -- be curious to see what jason thinks.

you really don't want to use printf for wrtmessage, since it will malloc.  i'd either not implement the function or use write as it does there.

if you need to implement abort, please move it up to with the other windows specific functions (ffs, etc).  Also check your indentation and style (empty line at the beginning of the function if no variables).  same with errno and friends.

given the number of things that are MOZ_MEMORY_WINDOWS || MOZ_MEMORY_WINCE, it might make sense to #define MOZ_MEMORY_WINDOWS for wince and && !wince the few exceptions

i still think that the chunk_alloc_mmap stuff needs to be made more generic, so that function doesn't have a bunch of direct wince calls in the middle of it.  i don't see any reason why we can't do what you're doing there on windows itself.

you've got some scattered printfs and debug code that should be removed

r? from jason for additional comments
Comment 12 Doug Turner (:dougt) 2009-03-13 08:11:34 PDT
*** Bug 477956 has been marked as a duplicate of this bug. ***
Comment 13 Brad Lassey [:blassey] (use needinfo?) 2009-03-17 13:03:51 PDT
Created attachment 367834 [details] [diff] [review]
patch v.2

a couple comments/explanations:

vsnprintf seems to allocate on windows ce.  I've punted on malloc_printf, not sure if there is a way to make that work other than implementing it from scratch (and not sure that's worth it)

VirtualFree is interesting.  The documentation says you should pass 0 as the size if releasing the memory (MEM_RELEASE).  If you do that, a subsequent call to VirtualAlloc with that address will fail.  If you pass in the size, VirtualFree returns an error code....but the subsequent call to VirtualAlloc will succeed.  This patch passes in the size and ignores the error code that gets returned.

Another interesting point is that VirtualQuery shows no change to the state of the memory after VirtualFree regardless of the size passed in.
Comment 14 Stuart Parmenter 2009-03-17 23:37:30 PDT
Comment on attachment 367834 [details] [diff] [review]
patch v.2

i think this is mostly fine.  Jason, can you take a look?
Comment 15 Jason Evans 2009-03-20 11:23:54 PDT
Comment on attachment 367834 [details] [diff] [review]
patch v.2

The patch looks fine for the most part, but I'm concerned that there might be something fundamentally wrong about how the page mapping (VirtualAlloc() and pages_map() code) is working.  There is a change in chunk_alloc_mmap() that apparently handles non-chunk-aligned results from pages_map(), but that should never happen -- pages_map() should return NULL instead of returning mapped memory at the wrong address.  I can't say for sure that there's something wrong here, but it doesn't feel right...

Minor cruft: The char addr_buf[12] and char size_buf[12] variables do not appear to be used.
Comment 16 Brad Lassey [:blassey] (use needinfo?) 2009-03-20 11:39:15 PDT
(In reply to comment #15)
> (From update of attachment 367834 [details] [diff] [review])
> The patch looks fine for the most part, but I'm concerned that there might be
> something fundamentally wrong about how the page mapping (VirtualAlloc() and
> pages_map() code) is working.  There is a change in chunk_alloc_mmap() that
> apparently handles non-chunk-aligned results from pages_map(), but that should
> never happen -- pages_map() should return NULL instead of returning mapped
> memory at the wrong address.  I can't say for sure that there's something wrong
> here, but it doesn't feel right...

Are you referring to this change?

-		while (ret == NULL) {
+		while (ret == NULL  || CHUNK_ADDR2BASE(ret) != ret) {
 
That was a condition I was hitting, but it was a bug in my code which I fixed.  I figured it was still a good sanity check though.  If you don't like it, I'll remove it.
 
> Minor cruft: The char addr_buf[12] and char size_buf[12] variables do not
> appear to be used.

yes, thank you for catching that, I'll get rid of it before pushing.  I was using itoa to do some debugging output since the printf stuff doesn't work with wince.
Comment 17 Brad Lassey [:blassey] (use needinfo?) 2009-03-23 21:08:24 PDT
pushed http://hg.mozilla.org/mozilla-central/rev/20014cbebfb6
Comment 18 Brad Lassey [:blassey] (use needinfo?) 2009-03-24 14:13:23 PDT
Created attachment 369148 [details] [diff] [review]
extra assertion

I'm seeing some odd behavior in debug builds, adding this assertion corrects it.  I suspect this is a compiler issue.

Also, I've confirmed that this is not an issue with release builds.
Comment 19 Brad Lassey [:blassey] (use needinfo?) 2009-03-25 11:21:58 PDT
Created attachment 369316 [details] [diff] [review]
if ret != requested address, free ret and return null

this approach is less hocus pocus
Comment 20 Doug Turner (:dougt) 2009-03-26 14:31:39 PDT
guys, without the change brad mentioned:

-        while (ret == NULL) {
+        while (ret == NULL  || CHUNK_ADDR2BASE(ret) != ret) {


I crash on startup every time.  I am reopening this bug.  Jason/stuart/brad, any idea why we need this bandaide?
Comment 21 Doug Turner (:dougt) 2009-03-26 15:53:30 PDT
blassey, on wince we have a pattern that reserves then frees, etc.  without committing.  This is different than on windows where each call to virtualalloc is not only a reserve but a commit.  If some other caller (another thread even) is also calling VirtualAlloc, cause problems?

For example:

Thread 1: ret = VirtualAlloc(null, size);
Thread 1: ret = VirtualAlloc(ret, size);
Thread 1: VirtualFree(ret)
Thread 2: ret = VirtualAlloc(null, size);
Thread 2: ret = VirtualAlloc(ret, size);
Thread 1: ret = VirtualAlloc(ret, size);  // bad?

is there some magic locking that i didn't see?  maybe the crt patches for w32 provide this?


Also, just inspecting the code here and looking for deltas between w32 and wce:

Would it make sense to make a COMMIT call right after the call to reserve:
http://mxr.mozilla.org/mozilla-central/source/memory/jemalloc/jemalloc.c#2170

Doing this would also let you get rid of this explict call:
http://mxr.mozilla.org/mozilla-central/source/memory/jemalloc/jemalloc.c#2506
Comment 22 Brad Lassey [:blassey] (use needinfo?) 2009-03-30 02:17:26 PDT
Created attachment 369979 [details] [diff] [review]
reserves, but doesn't commit unaligned addersses
Comment 23 Brad Lassey [:blassey] (use needinfo?) 2009-03-30 14:27:34 PDT
Created attachment 370071 [details] [diff] [review]
patch v.4

first attempt to reserve size+offset, giving us an aligned area to commit and leaving us aligned for the next reservation.  Then fallback to reserving size+chunksize, which is guaranteed to give us an aligned area to commit.

in pages_unmap, first try to VirtualFree the address that's passed in.  In the case of error code 87 (invalid parameter), which is what is returned when you try to release an address which isn't the base of a reserved region, use VirtualQuery to get the base address and release that.
Comment 24 Brad Lassey [:blassey] (use needinfo?) 2009-03-30 20:57:49 PDT
Created attachment 370140 [details] [diff] [review]
patch v.5
Comment 25 Brad Lassey [:blassey] (use needinfo?) 2009-03-30 21:31:49 PDT
Created attachment 370143 [details] [diff] [review]
patch v.6
Comment 26 Stuart Parmenter 2009-03-30 21:52:54 PDT
Comment on attachment 370143 [details] [diff] [review]
patch v.6

I think this looks OK, and will make things a lot better than they are now.

My only real comment is you might want to swap the pfd and alignment params to  pages_map_align() to be consistent with the rest of the code.


Jason can you also take a look at this?  After digging for a while, the problem we have on Windows Mobile is that we can't VirtualAlloc at a specific address, so if passing NULL in as the address doesn't work, we can't VirtualFree it and try remapping in that same spot + alignment offset.  With this code in Brad's testing we're ending up aligning correctly most of the time after a while.  Unfortunately, if we have to over-reserve, we don't give back the start of the block that we need to VirtualFree it with :/.  We added some hacky code to our pages_unmap code to VirtualQuery to address if VirtualFree fails to find the base address and try freeing that.  I'm not super happy with this, but not sure what else we can do short of keeping the real address somewhere and refactoring a lot of code.  The perf hit here should be minimal and is windows mobile only, so maybe not the end of the world?

The other piece I'm not sure about is:
+#ifdef MOZ_MEMORY_WINCE
+		ret = pages_map_align(chunk_size, pfd, alignment);
+#else

Should that just be a JEMALLOC_USES_MAP_ALIGN?
Comment 27 Brad Lassey [:blassey] (use needinfo?) 2009-03-30 23:29:34 PDT
Stuart, you asked to get a log of our aligned vs. unaligned alloc's.  I ran the page load test from talos (3 pages, 5 times). 

1: aligned, over-alloc
2: aligned
3: unaligned, over-alloc
4: pages_map

Every pages_map_align starts with a pages_map, which is why the patter is 4 then 2.  None of the allocs in the run were over-allocated.

*******alloc******* 4
*******alloc******* 4
*******alloc******* 2
*******alloc******* 4
*******alloc******* 2
*******alloc******* 4
*******alloc******* 2
*******alloc******* 4
*******alloc******* 2
*******alloc******* 4
*******alloc******* 2
*******alloc******* 4
*******alloc******* 2
*******alloc******* 4
*******alloc******* 2
*******alloc******* 4
*******alloc******* 2
*******alloc******* 4
*******alloc******* 2
*******alloc******* 4
*******alloc******* 2
*******alloc******* 4
*******alloc******* 2
*******alloc******* 4
*******alloc******* 2
Comment 28 Jason Evans 2009-03-30 23:42:16 PDT
Comment on attachment 370143 [details] [diff] [review]
patch v.6

> static void *
> pages_map(void *addr, size_t size, int pfd)
> {
>-	void *ret;
>+	void *ret = NULL;
> #if defined(MOZ_MEMORY_WINCE)
>-	ret = VirtualAlloc(addr, size, MEM_RESERVE, PAGE_NOACCESS);
>+	void *va_ret;
>+	assert(addr != NULL);
>+	va_ret = VirtualAlloc(addr, size, MEM_RESERVE, PAGE_NOACCESS);
>+	if (va_ret)
>+		ret = VirtualAlloc(ret, size, MEM_COMMIT, PAGE_READWRITE);
                                   ^^^ va_ret ?
                                   ^^^^^^^^^^^^
>+	assert(va_ret == ret);
> #elif defined(MOZ_MEMORY_WINDOWS)
> 	ret = VirtualAlloc(addr, size, MEM_COMMIT | MEM_RESERVE,
> 	    PAGE_READWRITE);
> #endif
> 	return (ret);
> }

-----------------------------------------------------------------------------

> 	do {
>+#ifdef MOZ_MEMORY_WINCE
>+		ret = pages_map_align(chunk_size, pfd, alignment);
>+#else

This should go just outside the do..while loop, since as written it will cause an infinite loop if pages_map_align() fails.

-----------------------------------------------------------------------------

> [comment #26] Should that just be a JEMALLOC_USES_MAP_ALIGN?

Yes, it looks to me like that would be an improvement.

Overall, this patch looks good to me.
Comment 29 Brad Lassey [:blassey] (use needinfo?) 2009-03-31 09:37:09 PDT
(In reply to comment #28)

> >+	assert(addr != NULL);
fwiw, this should be addr == NULL.  Fixed locally.  Also, the adjustment of size came after the pages_map, fixed that as well.

> >+	va_ret = VirtualAlloc(addr, size, MEM_RESERVE, PAGE_NOACCESS);
> >+	if (va_ret)
> >+		ret = VirtualAlloc(ret, size, MEM_COMMIT, PAGE_READWRITE);
>                                    ^^^ va_ret ?
>                                    ^^^^^^^^^^^^
yes, fixed locally as well.

> -----------------------------------------------------------------------------
> 
> > 	do {
> >+#ifdef MOZ_MEMORY_WINCE
> >+		ret = pages_map_align(chunk_size, pfd, alignment);
> >+#else
> 
> This should go just outside the do..while loop, since as written it will cause
> an infinite loop if pages_map_align() fails.
> 
done
> -----------------------------------------------------------------------------
> 
> > [comment #26] Should that just be a JEMALLOC_USES_MAP_ALIGN?
> 
> Yes, it looks to me like that would be an improvement.
done

pushed http://hg.mozilla.org/mozilla-central/rev/865b0dbcf1a3
Comment 30 Hiroyuki Ikezoe (:hiro) 2009-04-01 16:23:38 PDT
Created attachment 370534 [details] [diff] [review]
A patch

Brad, I think VirtualFree with MEM_DECOMMIT should be called before invoking VirtualFree with MEM_RELEASE in pages_unmap since VirtualAlloc with MEM_COMMIT is invoked in pages_map.
Comment 31 Brad Lassey [:blassey] (use needinfo?) 2009-04-01 17:28:19 PDT
Comment on attachment 370534 [details] [diff] [review]
A patch

From the VirtualFree documentation, in the section describing MEM_RELEASE:

"The function does not fail if you attempt to release pages that are in different states, some reserved and some committed. This means that you can release a range of pages without first determining the current commitment state."
Comment 32 Hiroyuki Ikezoe (:hiro) 2009-04-01 17:44:45 PDT
(In reply to comment #31)
> (From update of attachment 370534 [details] [diff] [review])
> From the VirtualFree documentation, in the section describing MEM_RELEASE:
> 
> "The function does not fail if you attempt to release pages that are in
> different states, some reserved and some committed. This means that you can
> release a range of pages without first determining the current commitment
> state."

I guess the document you mention is not for Windows Mobile.

I think VirtualFree document for Windows Mobile is http://msdn.microsoft.com/en-us/library/aa908947.aspx.
Comment 33 Brad Lassey [:blassey] (use needinfo?) 2009-04-01 18:31:45 PDT
(In reply to comment #32)
> I guess the document you mention is not for Windows Mobile.
> 
> I think VirtualFree document for Windows Mobile is
> http://msdn.microsoft.com/en-us/library/aa908947.aspx.

Yes, I saw that before and tested to confirm since the docs have been so wrong in so many places.  VirtualFree is not returning an error code when freeing a range of memory with mixed states.  Also, after being freed that memory does get returned in subsequent calls to VirtualAlloc.  

If we were going to program to the docs (which is entirely reasonable), I would put the VirtualFree(addr, size, MEM_DECOMMIT) just before the VirtualQuery because there is a cost associated with these API calls and that is the only place where we should encounter ranges of memory of mixed states.
Comment 34 Stuart Parmenter 2009-04-01 19:31:41 PDT
(In reply to comment #30)
> Created an attachment (id=370534) [details]
> A patch
> 
> Brad, I think VirtualFree with MEM_DECOMMIT should be called before invoking
> VirtualFree with MEM_RELEASE in pages_unmap since VirtualAlloc with MEM_COMMIT
> is invoked in pages_map.

Why do you think that?  I've got a small testcase that only uses MEM_RELEASE that can continue to allocate forever in a loop.
Comment 35 Hiroyuki Ikezoe (:hiro) 2009-04-01 20:23:34 PDT
(In reply to comment #34)
> (In reply to comment #30)
> > Created an attachment (id=370534) [details] [details]
> > A patch
> > 
> > Brad, I think VirtualFree with MEM_DECOMMIT should be called before invoking
> > VirtualFree with MEM_RELEASE in pages_unmap since VirtualAlloc with MEM_COMMIT
> > is invoked in pages_map.
> 
> Why do you think that?  I've got a small testcase that only uses MEM_RELEASE
> that can continue to allocate forever in a loop.

As far as I confirmed, without DECOMMIT, AllocationProtect obtained by VirtualQuery keeps sometimes PAGE_READWRITE after invoking VirtualFree with MEM_RELEASE.

I think this will cause something badness.
Comment 36 Hiroyuki Ikezoe (:hiro) 2009-04-01 20:38:44 PDT
(In reply to comment #35)
> (In reply to comment #34)
> > (In reply to comment #30)
> > > Created an attachment (id=370534) [details] [details] [details]
> > > A patch
> > > 
> > > Brad, I think VirtualFree with MEM_DECOMMIT should be called before invoking
> > > VirtualFree with MEM_RELEASE in pages_unmap since VirtualAlloc with MEM_COMMIT
> > > is invoked in pages_map.
> > 
> > Why do you think that?  I've got a small testcase that only uses MEM_RELEASE
> > that can continue to allocate forever in a loop.
> 
> As far as I confirmed, without DECOMMIT, AllocationProtect obtained by
> VirtualQuery keeps sometimes PAGE_READWRITE after invoking VirtualFree with
> MEM_RELEASE.
> 
> I think this will cause something badness.

Oops, I was wrong. I watched Protect, and MSDN says Protect is undefined with 
MEM_FREE state.
Comment 37 Mike Beltzner [:beltzner, not reading bugmail] 2009-04-15 11:58:44 PDT
Comment on attachment 367834 [details] [diff] [review]
patch v.2

please provide a rolled up 191 patch in bug 485227
Comment 38 Brad Lassey [:blassey] (use needinfo?) 2009-04-15 12:20:16 PDT
Created attachment 372934 [details] [diff] [review]
rolled up patch
Comment 39 Mike Beltzner [:beltzner, not reading bugmail] 2009-04-15 12:35:52 PDT
Comment on attachment 372934 [details] [diff] [review]
rolled up patch

a191=beltzner
Comment 40 Aki Sasaki [:aki] 2009-04-15 19:23:04 PDT
Created attachment 373048 [details] [diff] [review]
[checked in] add to mozconfig
Comment 41 Aki Sasaki [:aki] 2009-04-15 19:36:40 PDT
Comment on attachment 373048 [details] [diff] [review]
[checked in] add to mozconfig

revision 1095:ac0f4c2e7058
should get picked up on the next build.
Comment 42 Brad Lassey [:blassey] (use needinfo?) 2009-04-15 22:20:53 PDT
pushed http://hg.mozilla.org/releases/mozilla-1.9.1/rev/8274e63ba0a5
Comment 43 Brian Crowder 2009-04-16 07:53:33 PDT
Sorry for late drive-by, but the rolled-up patch seems to have 8-character indentation and tabs in some spots.

Note You need to log in before you can comment on or make changes to this bug.