Open Bug 1356594 Opened 8 years ago Updated 2 years ago

Rewrite AppConstants.jsm in C++

Categories

(Toolkit :: General, defect, P3)

Product:
Toolkit
Component:
General
Type:
defect

Tracking

()

Project Flags:
Performance Impact low

People

(Reporter: mccr8, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: perf:resource-use, Whiteboard: [overhead:50k])

AppConstants.jsm defines a bunch of build-specific constants. In addition to the 30kb to 50kb or whatever jsm overhead, there's about a thousand characters of properties and 100 characters of data. All of this is static, so it should be possible to share it between all processes if it was written in C++. (I don't know if XPIDL or WebIDL makes more sense.) The code is really trivial so there shouldn't be much danger of introducing hazards with it. I started looking at it, but I'm not sure how to deal with the @MOZ_APP_VERSION@-type of macro expansion. At least some of these @MACROS@ appear to be used as regular C macros in some files, but I don't know what defines them.
(In reply to Andrew McCreight [:mccr8] from comment #0) > AppConstants.jsm defines a bunch of build-specific constants. In addition to > the 30kb to 50kb or whatever jsm overhead, there's about a thousand > characters of properties and 100 characters of data. All of this is static, > so it should be possible to share it between all processes if it was written > in C++. (I don't know if XPIDL or WebIDL makes more sense.) The code is > really trivial so there shouldn't be much danger of introducing hazards with > it. This is a great idea! > I started looking at it, but I'm not sure how to deal with the > @MOZ_APP_VERSION@-type of macro expansion. At least some of these @MACROS@ > appear to be used as regular C macros in some files, but I don't know what > defines them. They're defined via DEFINES in moz.build and the build system, e.g.: http://dxr.mozilla.org/mozilla-central/source/toolkit/modules/moz.build#264
Florian was also interested in inefficient things we expose to JS; this is definitely one of those things. His rewrite-all-the-JS experience might come in handy here as well.
Another example of moz.build stuff needed to define these constants that Nathan pointed out to me in IRC is: for var in ('ANDROID_PACKAGE_NAME', 'MOZ_APP_NAME', 'MOZ_APP_VERSION', 'MOZ_APP_VERSION_DISPLAY', 'MOZ_MACBUNDLE_NAME', 'MOZ_WIDGET_TOOLKIT', 'DLL_PREFIX', 'DLL_SUFFIX', 'DEBUG_JS_MODULES'): DEFINES[var] = CONFIG[var]
What's the exact criteria that we use here? When I was working on bug 1250473 I was wondering whether it should be written in C++ also because it seemed like we could simplify a ton of things if we didn't have so many layers of abstractions between Gecko and this bit of toolkit code...
AppConstants.jsm is also one of the latest JS files making use of the preprocessor, it would be nice to get rid of that.
(In reply to Andrew McCreight [:mccr8] from comment #0) > All of this is static, so it should be possible to share it between all > processes if it was written in C++. I'd be surprised if we managed to share very much between processes. Most of the ways we usually reflect things like this to JS still require creating all of the same properties on the JS reflector or its prototypes. And if we generally wind up creating separate reflectors for each compartment, so if we're not careful, we could actually wind up using more memory... We could probably avoid some of that by natively resolving properties each time, rather than defining them on reflectors, but that could potentially hurt performance a lot. And for string properties, we'd have to be careful not to create new JS strings for every lookup.
Whiteboard: [fxperf]
Priority: -- → P3
Flags: needinfo?(gijskruitbosch+bugs)
Whiteboard: [fxperf] → [fxperf:p3]
Whiteboard: [fxperf:p3] → [fxperf:p3][overhead:50k]

Gijs, is this still a good idea and something you'd like to follow up on?

(In reply to Jared Hirsch [:jhirsch] [:_6a68] (Needinfo please) from comment #7)

Gijs, is this still a good idea and something you'd like to follow up on?

I think it's still a good idea but it's complex enough (and I'm busy enough) that I'm unlikely to do it myself. The comments have a good overview of some of the complexity here. In addition to what's mentioned here, all the various modules and JS files importing the module isn't free, so fixing that, too, would be a boon. If the strings are a problem for memory use, perhaps we could replace the AppConstants platform strings with separate properties (AppConstants.XP_WIN and such) that would be bool.

Performance Impact: --- → P3
Flags: needinfo?(gijskruitbosch+bugs)
Keywords: perf:resource-use
Whiteboard: [fxperf:p3][overhead:50k] → [overhead:50k]
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.