Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mozjs52: update to 52.9.0. #2348

Closed
wants to merge 1 commit into from
Closed

Conversation

Cogitri
Copy link
Contributor

@Cogitri Cogitri commented Sep 3, 2018

No description provided.

@@ -12,7 +12,7 @@ homepage="http://www.mozilla.org/js/"
license="MPL-1.1, GPL-2, LGPL-2.1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use SPDX short-identifiers

@Cogitri Cogitri force-pushed the mozjs-52-bump branch 2 times, most recently from 01e3fd4 to 6a734a1 Compare September 4, 2018 17:44
@Cogitri
Copy link
Contributor Author

Cogitri commented Sep 4, 2018

This also enables jemalloc for glibc builds, since jemalloc's performance is still better than glibc's malloc. Should be tested once icu rebuilding has been done.

@maxice8
Copy link
Contributor

maxice8 commented Sep 7, 2018

You will need this patch, put it in the patches subdirectory with a .patch prefix.

--- memory/mozjemalloc/jemalloc.c	2018-09-07 01:30:40.056777558 -0300
+++ memory/mozjemalloc/jemalloc.c	2018-09-07 01:30:45.847777478 -0300
@@ -331,9 +331,6 @@
 #endif
 #include <sys/time.h>
 #include <sys/types.h>
-#if !defined(MOZ_MEMORY_SOLARIS) && !defined(MOZ_MEMORY_ANDROID)
-#include <sys/sysctl.h>
-#endif
 #include <sys/uio.h>
 #ifndef MOZ_MEMORY
 #include <sys/ktrace.h> /* Must come after several other sys/ includes. */

@Gottox
Copy link
Member

Gottox commented Sep 7, 2018

[...] since jemalloc's performance is still better than glibc's malloc.

Are there any (real world) benchmarks that support that statement? The performance of memory allocation heavily depends on the workload. And even then I don't see where mozjs52 is used in performance critical packages.

Allows jemalloc to be used in conjunction with glibc malloc? What if mozjs52 allocates memory and the application uses the libc's free()? Does this work?

@Cogitri
Copy link
Contributor Author

Cogitri commented Sep 7, 2018

Are there any (real world) benchmarks that support that statement?

I can't seem to find benchmarks for spidermonkey with and without jemalloc right now, but other packages do have rather big performance improvements.

Also, Mozilla has been using jemalloc by default for years now, so I feel like it's a good idea if we use it as well.

I don't see where mozjs52 is used in performance critical packages.

Hm, yeah, once gjs 1.54.0 is in the tree the only packages consuming it seem to be cjs,goffie&polkit. If you don't want to pull in the dep for that that's fine by me

@maxice8
Copy link
Contributor

maxice8 commented Sep 18, 2018

I think it is better we just not enable jemalloc for now

@maxice8
Copy link
Contributor

maxice8 commented Sep 18, 2018

Pushed a fixed version of your commit:

  • Removed patch that only apply to jeamalloc and was failing because patch-args is '-Np1' but the patch itself is meant for '-p0'

@maxice8 maxice8 closed this Sep 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants