Closed Bug 488608 Opened 15 years ago Closed 15 years ago

enable jemalloc on CE6

Categories

(Core :: Memory Allocator, defect)

ARM
Windows CE
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: vlad, Assigned: vlad)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch jemalloc on ce6 (obsolete) — Splinter Review
jemalloc doesn't need some of the workarounds for CE >= 6; also, this patch cleans up some of the linkage issues and has all the malloc symbols exported directly from jemalloc through the shunt, instead of going through a separate step in the shunt.
Attachment #373021 - Flags: review?(pavlov)
Comment on attachment 373021 [details] [diff] [review]
jemalloc on ce6

As discussed on irc, I think it would be cleaner to export these symbols from the shunt's module definition file rather than have MOZ_MEMORY_EXPORT
Attachment #373021 - Flags: review?(bugmail) → review-
Updated; this depends on the shunt cleanup in bug 489487 and the addition of the DEF file.

This exports all jemalloc symbols through the def file, and does the same thing for the other memory-allocating symbols that we override (_strndup etc.).  We trust that the same mechanism that we use to get our own allocator will give us the right symbols for the string functions.
Attachment #373021 - Attachment is obsolete: true
Attachment #373968 - Flags: superreview?(pavlov)
Attachment #373968 - Flags: review?(bugmail)
Attachment #373021 - Flags: review?(pavlov)
Attachment #373968 - Flags: superreview?(pavlov) → superreview+
Comment on attachment 373968 [details] [diff] [review]
updated
[Checkin: Comment 4]

for my sake, can you:
get rid of the jemalloc_bool stuff and just #define it when necessary

in ffs() leave an empty first line in the function (to be consistent with the rest of the file, if there is no variable declarations)

also, get rid of the changes to:
-void	*malloc(size_t size);
+void* malloc(size_t size);


+#else /* WIN CE */

make this more explicit?

otherwise this looks fine
Attachment #373968 - Flags: review?(bugmail) → review+
http://hg.mozilla.org/mozilla-central/rev/e6675b1c79a8
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
I got an error with clean build.

Building Windows CE Shunt Library and Tool Chain
Using SDK in:
c:/program files/windows mobile 6 sdk/pocketpc
make[3]: Entering directory `/c/cygwin/home/user/hg/mozilla-central/build/wince/
tools'
make[3]: *** No rule to make target `/c/cygwin/home/user/hg/mozilla-central/objd
ir-wm6/xulrunner//c/cygwin/home/user/hg/mozilla-central/build/wince/tools/arm-wi
nce-as.c', needed by `c:/cygwin/home/user/hg/mozilla-central/objdir-wm6/xulrunne
r/dist/sdk/bin/arm-wince-as.exe'.  Stop.
Comment on attachment 373968 [details] [diff] [review]
updated
[Checkin: Comment 4]

>diff -r ab828eaacae1 configure.in
>--- a/configure.in	Tue Apr 21 10:25:44 2009 -0700
>+++ b/configure.in	Tue Apr 21 17:15:39 2009 -0700
>@@ -268,7 +268,7 @@
> 
>     _pwdw=`pwd -W`
>     _pwd=`pwd`
>-    make WINCE_SDK_DIR="$WINCE_SDK_DIR" TOPSRCDIR="$srcdir" OBJDIR="$_pwdw" -C $srcdir/build/wince/tools
>+    make WINCE_SDK_DIR="$WINCE_SDK_DIR" TOPSRCDIR="$_pwd/$srcdir" OBJDIR="$_pwdw" -C $srcdir/build/wince/tools

Is this change really needed?
interesting.. it is, but I guess it needs to be smarter -- as it was before, it was broken for relative source dirs.  That probably shouldn't have come in as part of this patch though.  Looks like we need to make TOPSRCDIR an absolute pathname somehow before passing it in.
$_topsrcdir is an absolute path. We can use this value?
Is it an absolute path because you're using an absolute path to call configure, or is it always an absolute path?  Try calling configure as ../../wherever/configure
Yes, it works fine since $_topsrcdir is defined with pwd. 

http://mxr.mozilla.org/mozilla-central/source/configure.in#151


A part of configure log:

user@DEWINDOWSXP ~/hg/objdir-wm6/xulrunner
$ ../../mozilla-central/configure
Adding configure options from ../../mozilla-central/.mozconfig:
  --enable-debug
  --disable-optimize
  --enable-plugins
  --enable-xpinstall
  --enable-timeline
  --enable-installer
  --enable-jemalloc
  --disable-tests
  --disable-javaxpcom
  --disable-accessibility
  --disable-printing
  --disable-oji
  --disable-vista-sdk-requirements
  --disable-updater
  --enable-image-decoders=png gif jpeg bmp icon
  --disable-dbm
  --disable-ogg
  --target=arm-wince
  --enable-win32-target=WINCE
  --enable-default-toolkit=cairo-windows
  --with-wince-sdk=c:/program files/windows mobile 6 sdk/pocketpc
creating cache ./config.cache
checking host system type... i686-pc-mingw32
checking target system type... arm-unknown-wince
checking build system type... i686-pc-mingw32
checking for mawk... no
checking for gawk... gawk
checking for perl5... no
checking for perl... /bin/perl
-----------------------------------------------------------------------------
Building Windows CE Shunt Library and Tool Chain
Using SDK in:
c:/program files/windows mobile 6 sdk/pocketpc
make: Entering directory `/c/cygwin/home/user/hg/mozilla-central/build/wince/too
ls'
mkdir -p c:/cygwin/home/user/hg/objdir-wm6/xulrunner/dist/sdk/bin;
cl -O2 -DVC_PATH='"C:\\Program Files\\Microsoft Visual Studio 9.0\\VC\\"' -DWM_S
DK_PATH='"c:/program files/windows mobile 6 sdk/pocketpc\\"' -DMOZCE_DEVENV='"vs
9"' -DTOPSRCDIR='"/c/cygwin/home/user/hg/mozilla-central"'  -DWIN_SDK_PATH='"C:\
\Program Files\\Microsoft SDKs\\Windows\\v6.0A\\"' -DEBUG -Zi -Foc:/cygwin/home/
user/hg/objdir-wm6/xulrunner/dist/sdk/bin -Fec:/cygwin/home/user/hg/objdir-wm6/x
ulrunner/dist/sdk/bin/arm-wince-as.exe /c/cygwin/home/user/hg/mozilla-central/bu
ild/wince/tools/arm-wince-as.c
Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 15.00.30729.01 for 80x86
Copyright (C) Microsoft Corporation.  All rights reserved.

arm-wince-as.c
Microsoft (R) Incremental Linker Version 9.00.30729.01
Copyright (C) Microsoft Corporation.  All rights reserved.
Comment on attachment 374709 [details] [diff] [review]
Use $_topsrcdir
[Duplicate of bug 490325]

perfect then -- I didn't know we had it available.  If someone doesn't beat me to it, I can check this in tomorrow morning.
Attachment #374709 - Flags: review+
Attachment #373968 - Attachment description: updated → updated [Checkin: Comment 4]
Comment on attachment 374709 [details] [diff] [review]
Use $_topsrcdir
[Duplicate of bug 490325]


http://hg.mozilla.org/mozilla-central/rev/0db07277c203
Attachment #374709 - Attachment description: Use $_topsrcdir. → Use $_topsrcdir [Checkin: Comment 13]
Attachment #374709 - Attachment description: Use $_topsrcdir [Checkin: Comment 13] → Use $_topsrcdir [Duplicate of bug 490325]
Attachment #374709 - Attachment is obsolete: true
(In reply to comment #13)
> http://hg.mozilla.org/mozilla-central/rev/0db07277c203

Damn, this was bug 490325...
Target Milestone: --- → mozilla1.9.2a1
Version: unspecified → Trunk
Depends on: 490325
You need to log in before you can comment on or make changes to this bug.