Mozilla 2: Outparamdel

There will be a lot of under-the-hood code changes in Mozilla 2. Our goal is to end up with a simpler, safer and faster codebase.

This is my perspective on the work ahead with respect to outparamdel.

Outparamdel

In the presence of a garbage collector we will be getting rid of stack nsCOMPtr<> usage (using raw pointers instead), but the getter_Addrefs() will still be needed to pass references to heap-allocated nsCOMPtr so they can be assigned to. Eliminating as many outparams as possible will eliminate much getter_Addrefs() footprint/perf/code bloat.

Within the next week I hope to attempt to rewrite all of the auto-detectable outparamdel candidates.

Determining What To Rewrite

outparams.js is a dehydra script for flagging code that can be rewritten to use outparameters. It turned out a bit trickier than I initially expected. Here are the checks required

  1. Check that a function only ever returns 1 failure error code (NS_OK and something else). This enough if a function is not virtual.
  2. Ensure that either a) This is the only implementation of a particular virtual function or b) All other implementations of this function satisfy 1
    Currently I only do a).
  3. Also check that all overloads of this function have the same outparameter type. This is required since C++ (thankfully) doesn’t not allow function overloading by varying the return type.
  4. Checks 1-3 ensure that the function can be rewritten, however one also needs to determine if the return type should be wrapped in getter_Addrefs<>. This can not be deterministically done from looking at the getter. So one has to scan the code for usage of the function to see if the outparam is ever passed getter_Addrefs.
  5. Check that none of the callers are within macros to minimize non-automatic rewriting.

Checks 2 and 3 require the complete class hierachy of Mozilla so I finally made a few more dehydra scripts to produce that. This was on my TODO list for a while and should make a few other interesting analyses possible (my favourite one is finding interfaces which only have 1 implementation to get rid of excessive virtual functions).

Checks 4 and 5 were easiest to implement as warnings in outparamdel.

One should keep in mind transitivity. Once the first outparamdel candidates are rewritten, some of their callers should become flagged for rewriting in the same manner.

Rewriting Code

Here is an example of how code will be simplified.

given a function:

nsresult getFoo(nsIFoo **out);

And usage like:

nsCOMPtr<nsIFoo> bla;
nsresult rv = getFoo(getter_Addrefs(bla));

if ((NS_SUCCEEDED(rv) && bla) {
...
} else {
return rv;
}
nsCOMPtr<nsIFoo> bla2;
return getFoo(getter_Addrefs(bla2));

The function definition will become:

nsIFoo* getFoo();

Before this can be done, several issues come up

  1. It is not clear if the original getFoo() is allowed to always override the value passed to it. This is hard to determine automatically, so we make the assumption that in the general case it is ok.
  2. It isn’t obvious if getFoo() is returns null in the outparam to indicate some non-error condition. This is rare so the parameter shall be annotated. Currently, the plan is to annotate with a NULLABLE_OUTPARAM() macro which would be detectable by dehydra and serve as documentation for the function behavior.

nsCOMPtr<nsIFoo> bla = getFoo(); //after XPCOMGC rewrite the left side will become nsIFoo* bla
if (bla) { // could even merge the above declaration into the if condition
...
} else {
return NS_ERROR;// or NULL if this is another outparamdel candidate
}
nsCOMPtr<nsIFoo> bla2 = getFoo();
return bla2 ? NS_OK : NS_ERROR_SOMETHING; // I'm not sure if outparamdel should use an explicit ternary operator or an inline function to convert new style errors into nsresult

Currently the code is being rewritten in a much uglier way. So this cleaner version will likely be implemented as an optimization pass (probably with a new tool outparamdel-opt?). There several tricks here:

  1. Connect the declaration with initialization of bla and bla2
  2. Detect the error check and replace it with “bla”(or !bla for NS_FAILED).Then realize that bla && bla contains a redundant statement and take it out.
  3. Do something similar to return statements.

Result

This should result in prettier code that compiles quicker and to a smaller, more efficient binary. It will also be more GC-friendly.

C++ Exceptions

Right now outparamdel does rewrites that are useful even if C++ exceptions will not be introduced. There are further code reduction gains possible if above error checks were converted into C++ exceptions, but I am not clear on performance characteristics of exceptions. We would also need to change tamarin exceptions to match C++ ones before any experimentation can be done.

4 comments

  1. How do you plan to handle the (very common) case where a function has two or more return values? (Think XPIDL arrays, for example.)

  2. I believe there’s a misunderstanding in Joe Drew’s comment: “C++ allows covariant return types in overloaded functions”. I believe the statement should read: ” … in overridden functions”. Hence, this does not apply to Tara’s situation (which actually isn’t an assumption: See Bjarne Stroustrup: “The C++ Programming language 3rd edition”, section 7.4.1, p. 151)
    )

  3. “C++ allows covariant return types in overloaded functions”