980227 - Reorder functions at link time

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: GonkIntegration, defect)

Description

[Ting-Yuan Huang]

The linker of ICS does little about programs' spatial locality. Because memory is paged, the working set can be unnecessarily large and harm performances of low-end devices dramatically (see bug 974308). This bug is opened to find out a quick way to tackle the problem. In an early experiment, the RSS of libxul.so's RO sections of a chrome/content process reduced from 10MB/13MB to 9MB/9MB respectively. There are some problems remaining, though. 

First, the ordering of codes are based on a profiling result. This means that we would need an automatic way to generate the profile regularly. We may also have to standardize a performance test for the profiler to run.

Second, the linker(gold) of ICS doesn't support "--section-ordering-file" but does in newer versions, so the experiment is conducted by using JB's toolchain to compile gecko. It can be risky to change or even mix different versions of toolchains. Another approach is to specify the orders in a gigantic linker script.

There are two bugs doing similar:
* Bug 603370 is for different purpose and platforms.
* Bug 970175 relies on the incoming gcc 4.9 which is unlikely to be default to Firefox OS shortly.

Comment 1

[Mike Hommey [:glandium]]

Note the infrastructure added in bug 603370 can be used here.

Comment 2

[Ting-Yu Chou [:ting] (away)]

(In reply to Ting-Yuan Huang from comment #0)
> First, the ordering of codes are based on a profiling result. This means
> that we would need an automatic way to generate the profile regularly. We
> may also have to standardize a performance test for the profiler to run.

Ting-Yuan thinks it is also fine to build the image in 2 rounds manually:

  1. build and generate profiling data
  2. specify the path of profiling results and build again

Note the profiling results Ting-Yuan mentioned are from gprof, not gcc's built-in profile feature (the "profiledbuild" we have now).

I will check bug 603370 for what have been done earlier, see if things can be reused.

Comment 3

[Mike Hommey [:glandium]]

Bug 603370 added all the infrastructure to reorder binaries when linking them, provided a list of symbols. All the details of how to give the list to the linker is handled by that infrastructure. It supports both gold and ld.bfd. It also figures things out with ICF. You just need to add a SYMBOL_ORDER variable to the Makefile.in corresponding to the lib to reorder pointing to the list file.

Comment 4

[Ting-Yu Chou [:ting] (away)]

Mike, this is awesome. But I don't quite understand "putting reordered stuff at the end" of bug 603370 comment 22, what's wrong with it?

Comment 5

[Ting-Yu Chou [:ting] (away)]

(In reply to Ting-Yu Chou [:ting] from comment #4)
> Mike, this is awesome. But I don't quite understand "putting reordered stuff
> at the end" of bug 603370 comment 22, what's wrong with it?

Ting-Yuan just explained me its because it is archived in APK for bug 603370, but not the case we will face for Firefox OS.

Comment 6

[Mike Hommey [:glandium]]

(In reply to Ting-Yu Chou [:ting] from comment #4)
> Mike, this is awesome. But I don't quite understand "putting reordered stuff
> at the end" of bug 603370 comment 22, what's wrong with it?

You don't have to worry about that, it was a problem with the previous patch from comment 21 which made gold somehow put reordered stuff last instead of first in the library. Or something like that.

Just list your symbols in a file, put that file in SYMBOL_ORDER in a Makefile.in, and, assuming this wasn't broken in the meanwhile, you should be all set.

Comment 7

[Ting-Yuan Huang]

Mike, the gold we are using is too old to recognize the INSERT command in the linker script. I tried to switch to ld.bfd (LDFLAGS += -fuse-ld=bfd) but the b2g process failed to start. Before I dive into the bfd problem, do you have any ideas?

Comment 8

[Ting-Yu Chou [:ting] (away)]

Used arm-linux-androideabi-4.6 which supports |--section-ordering-file| instead, but received an error while linking libmozglue.so:

  error: read-only segment has dynamic relocations

Comment 9

[Ting-Yu Chou [:ting] (away)]

(In reply to Ting-Yu Chou [:ting] from comment #8)
> Used arm-linux-androideabi-4.6 which supports |--section-ordering-file|
> instead, but received an error while linking libmozglue.so:
> 
>   error: read-only segment has dynamic relocations

Ting-Yuan reminds me this can be avoid by adding |-Wl,-z,notext|.

Comment 10

[Ting-Yu Chou [:ting] (away)]

This is how I built symbols sorted libxul.so for Tarako:

  1. Add "SYMBOL_ORDER = /path/to/symbol/order" to gecko/toolkit/library/Makefile.in
  2. Comment out the "-Wl,-z,text" checking in gecko/configure.in
  3. Switch prebuilt/linux-x86/toolchain/arm-eabi-4.4.3 to arm-eabi-4.6
  4. Switch prebuilt/linux-x86/toolchain/arm-linux-androideabi-4.4.x to arm-linux-androideabi-4.6
  5. Patch external/icu4c/common/unicode/unistr.h

         UnicodeString::replace(int32_t start,
           ...
           UBool isError = FALSE;
           U16_APPEND(buffer, count, U16_MAX_LENGTH, srcChar, isError);
       +   (void) isError;
         ...
         UnicodeString::append(UChar32 srcChar) {
           ...
           UBool isError = FALSE;
           U16_APPEND(buffer, _length, U16_MAX_LENGTH, srcChar, isError);
       +   (void) isError;
  6. Run build.sh

# Not sure whether step 3 is necessary or not.

Comment 11

[Mike Hommey [:glandium]]

(In reply to Ting-Yu Chou [:ting] from comment #10)
>   2. Comment out the "-Wl,-z,text" checking in gecko/configure.in

This being required means there's something wrong with the toolchain that should be fixed, because text relocations are generally bad for many reasons.

Comment 12

[Ting-Yuan Huang]

(In reply to Ting-Yu Chou [:ting] from comment #10)
>   3. Switch prebuilt/linux-x86/toolchain/arm-eabi-4.4.3 to arm-eabi-4.6
>   4. Switch prebuilt/linux-x86/toolchain/arm-linux-androideabi-4.4.x to
> arm-linux-androideabi-4.6

There is an easier way to build gecko with other toolchain:
ac_add_options --with-gonk-toolchain-prefix=/PATH/TO/TOOLCHAIN/bin/arm-linux-androideabi-

Comment 13

[Ting-Yuan Huang]

How to run gprof on B2G:

1) Neither the toolchain nor bionic come with the runtime required by gprof so merely adding -pg will not work and the linker complains. Here is a copy of android-ndk-profiler which provides the required stuffs. Note that libxul.so contains too many symbols so a little bit changes are required:
    https://github.com/ting-yuan/android-ndk-profiler
Then, under the project's root directory
    ./build.sh android-ndk-profiler && \
    cp `find out -name android-ndk-profiler.a` android-ndk-profiler/libandroid-ndk-profiler.a

2) Use C version Skia instead of ARM assembly, which uses r7 and conflicts with gprof. See attachment.

3) Build gecko with gprof enabled. Add the following into gonk-misc/default-gecko-config
ac_add_options --disable-crashreporter
ac_add_options --disable-release
ac_add_options --enable-profiling
export CXXFLAGS="-pg ${CXXFLAGS} -fno-omit-frame-pointer"
export CFLAGS="-pg ${CFLAGS} -fno-omit-frame-pointer"
export LDFLAGS="${LDFLAGS} -L/home/tingyuan/B2G/inari/android-ndk-profiler"
export LIBS="${LIBS} -landroid-ndk-profiler"

4) Run. Insert/call monstart("libxul.so") to where you want to start profiling. Set the path of gmon.out to the environment variable CPUPROFILE. Insert/call moncleanup() to stop profiling. 

5) Analyze the result
5.1) Normal gprof output
    arm-linux-androideabi-gprof /PATH/TO/SYMBOLS/libxul.so gmon.out | less
5.2) function ordering suggested by gprof:
    arm-linux-androideabi-gprof -r /PATH/TO/SYMBOLS/libxul.so gmon.out > ordering_file

Comment 14

[Ting-Yuan Huang]

Attachment: SKIA: Disable assembly on ARM platforms

Comment 15

[Ting-Yuan Huang]

Attachment: SKIA: Disable assembly on ARM platforms

Comment 16

[BMO Automation]

Firefox OS is not being worked on