Open Bug 1356594 Opened 5 years ago Updated 2 months ago

Rewrite AppConstants.jsm in C++

Categories

(Toolkit :: General, defect, P3)

defect

Tracking

()

People

(Reporter: mccr8, Unassigned, NeedInfo)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [fxperf:p3][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]
You need to log in before you can comment on or make changes to this bug.