ANDROID: binder: change down_write to down_read

From v4:

From v3:

From v2:

From v1:

binder_update_page_range needs down_write of mmap_sem because vm_insert_page need to change vma->vm_flags to VM_MIXEDMAP unless it is set. However, when I profile binder working, it seems every binder buffers should be mapped in advance by binder_mmap. It means we could set VM_MIXEDMAP in binder_mmap time which is already hold a mmap_sem as down_write so binder_update_page_range doesn't need to hold a mmap_sem as down_write.

Android suffers from mmap_sem contention so let's reduce mmap_sem down_write.

Cc Joe Perches <[email protected]>

Cc Arve Hjønnevåg <[email protected]>

Cc Todd Kjos <[email protected]>

Cc Greg Kroah-Hartman <[email protected]>

Reviewed-by Martijn Coenen <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---
From v4:
* Fix typo and VM flags clear handling - Joe

From v3:
* Fix typo

From v2:
* Fix vma->flag setting - Arve

From v1:
* remove WARN_ON_ONCE - Greg
* add reviewed-by - Martijn

Martijn, I took your LGTM of v1 as Reviewed-by. If you don't like it or want to change it to acked-by, please, tell me.

 drivers/android/binder.c       | 4 +++-
 drivers/android/binder_alloc.c | 6 +++---
 2 files changed, 6 insertions(+), 4 deletions(-)

drivers/android/binder.c +3 -1

index 764b63a5aade..bb63e3b54e0c 100644
@@ -4722,7 +4722,9 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma)
                failure_string = "bad vm_flags";
                goto err_bad_arg;
        }
-       vma->vm_flags = (vma->vm_flags | VM_DONTCOPY) & ~VM_MAYWRITE;
+       vma->vm_flags |= VM_DONTCOPY | VM_MIXEDMAP;
+       vma->vm_flags &= ~VM_MAYWRITE;
+
        vma->vm_ops = &binder_vm_ops;
        vma->vm_private_data = proc;
 

drivers/android/binder_alloc.c +3 -4

index 5a426c877dfb..4f382d51def1 100644
@@ -219,7 +219,7 @@ static int binder_update_page_range(struct binder_alloc *alloc, int allocate,
                mm = alloc->vma_vm_mm;
 
        if (mm) {
-               down_write(&mm->mmap_sem);
+               down_read(&mm->mmap_sem);
                vma = alloc->vma;
        }
 

@@ -288,7 +288,7 @@ static int binder_update_page_range(struct binder_alloc *alloc, int allocate,
                /* vm_insert_page does not seem to increment the refcount */
        }
        if (mm) {
-               up_write(&mm->mmap_sem);
+               up_read(&mm->mmap_sem);
                mmput(mm);
        }
        return 0;

@@ -321,7 +321,7 @@ static int binder_update_page_range(struct binder_alloc *alloc, int allocate,
        }
 err_no_vma:
        if (mm) {
-               up_write(&mm->mmap_sem);
+               up_read(&mm->mmap_sem);
                mmput(mm);
        }
        return vma ? -ENOMEM : -ESRCH;
--

2.17.0.rc1.321.gba9d0f2565-goog

2018-03-29 14:54 GMT+08:00 Minchan Kim <[email protected]>:

Quoted 8 lines

binder_update_page_range needs down_write of mmap_sem because vm_insert_page need to change vma->vm_flags to VM_MIXEDMAP unless it is set. However, when I profile binder working, it seems every binder buffers should be mapped in advance by binder_mmap. It means we could set VM_MIXEDMAP in binder_mmap time which is already hold a mmap_sem as down_write so binder_update_page_range doesn't need to hold a mmap_sem as down_write.

Quoted 2 lines

Android suffers from mmap_sem contention so let's reduce mmap_sem down_write.

Hi, Minchan:

It seems there is performance regression of this patch.

Do you have some test result of android app launch time or binderThroughput?

Hi Ganesh,

Quoted 12 lines · Ganesh Mahendran

2018-03-29 14:54 GMT+08:00 Minchan Kim <[email protected]>:

Quoted 10 lines

binder_update_page_range needs down_write of mmap_sem because vm_insert_page need to change vma->vm_flags to VM_MIXEDMAP unless it is set. However, when I profile binder working, it seems every binder buffers should be mapped in advance by binder_mmap. It means we could set VM_MIXEDMAP in binder_mmap time which is already hold a mmap_sem as down_write so binder_update_page_range doesn't need to hold a mmap_sem as down_write.

Android suffers from mmap_sem contention so let's reduce mmap_sem down_write.

Quoted 3 lines

Hi, Minchan:

It seems there is performance regression of this patch.

You mean "This patch aims for solving performance regression" not "This patch makes performance regression"?

Quoted 3 lines

Do you have some test result of android app launch time or binderThroughput?

Unfortunately, I don't have any number. The goal is to reduce the number of call mmap_sem as write-side lock because it makes priority inversion of threads easily and that's one of clear part I spot that we don't need write-side lock.

Thanks.

2018-03-30 9:29 GMT+08:00 Minchan Kim <[email protected]>:

Quoted 15 lines

Hi Ganesh,

Quoted 12 lines · Ganesh Mahendran

2018-03-29 14:54 GMT+08:00 Minchan Kim <[email protected]>:

Quoted 10 lines

binder_update_page_range needs down_write of mmap_sem because vm_insert_page need to change vma->vm_flags to VM_MIXEDMAP unless it is set. However, when I profile binder working, it seems every binder buffers should be mapped in advance by binder_mmap. It means we could set VM_MIXEDMAP in binder_mmap time which is already hold a mmap_sem as down_write so binder_update_page_range doesn't need to hold a mmap_sem as down_write.

Android suffers from mmap_sem contention so let's reduce mmap_sem down_write.

Quoted 6 lines

Quoted 3 lines

Hi, Minchan:

It seems there is performance regression of this patch.

You mean "This patch aims for solving performance regression" not "This patch makes performance regression"?

After applying this patch in our devices, app launch time increases about 15% in average.
"This patch makes performance regression", yes, from the results, it is like this.

I will do more test of this patch.

Hi Ganesh,

Quoted 9 lines · Ganesh Mahendran

2018-03-29 14:54 GMT+08:00 Minchan Kim <[email protected]>:

Quoted 8 lines

binder_update_page_range needs down_write of mmap_sem because vm_insert_page need to change vma->vm_flags to VM_MIXEDMAP unless it is set. However, when I profile binder working, it seems every binder buffers should be mapped in advance by binder_mmap. It means we could set VM_MIXEDMAP in binder_mmap time which is already hold a mmap_sem as down_write so binder_update_page_range doesn't need to hold a mmap_sem as down_write.

Quoted 6 lines

Quoted 2 lines

Android suffers from mmap_sem contention so let's reduce mmap_sem down_write.

Hi, Minchan:

It seems there is performance regression of this patch.

You mean "This patch aims for solving performance regression" not "This patch makes performance regression"?

Quoted 1 line

Quoted 1 line

Do you have some test result of android app launch time or binderThroughput?

Unfortunately, I don't have any number. The goal is to reduce the number of call mmap_sem as write-side lock because it makes priority inversion of threads easily and that's one of clear part I spot that we don't need write-side lock.

Quoted 25 lines · Minchan Kim

Hi Ganesh,

Quoted 9 lines · Ganesh Mahendran

2018-03-29 14:54 GMT+08:00 Minchan Kim <[email protected]>:

Quoted 8 lines

binder_update_page_range needs down_write of mmap_sem because vm_insert_page need to change vma->vm_flags to VM_MIXEDMAP unless it is set. However, when I profile binder working, it seems every binder buffers should be mapped in advance by binder_mmap. It means we could set VM_MIXEDMAP in binder_mmap time which is already hold a mmap_sem as down_write so binder_update_page_range doesn't need to hold a mmap_sem as down_write.

Quoted 6 lines

Quoted 2 lines

Android suffers from mmap_sem contention so let's reduce mmap_sem down_write.

Hi, Minchan:

It seems there is performance regression of this patch.

You mean "This patch aims for solving performance regression" not "This patch makes performance regression"?

Quoted 2 lines

Do you have some test result of android app launch time or binderThroughput?

Quoted 3 lines

Unfortunately, I don't have any number. The goal is to reduce the number of call mmap_sem as write-side lock because it makes priority inversion of threads easily and that's one of clear part I spot that we don't need write-side lock.

Please always run the binderThroughput tests when making binder changes (there is a binder test suite in the CTS Android tests), as that ensures that you are not causing performance regressions as well as just normal bug regressions :)

Quoted 26 lines · Minchan Kim

Quoted 25 lines

Hi Ganesh,

Quoted 9 lines · Ganesh Mahendran

2018-03-29 14:54 GMT+08:00 Minchan Kim <[email protected]>:

Quoted 8 lines

binder_update_page_range needs down_write of mmap_sem because vm_insert_page need to change vma->vm_flags to VM_MIXEDMAP unless it is set. However, when I profile binder working, it seems every binder buffers should be mapped in advance by binder_mmap. It means we could set VM_MIXEDMAP in binder_mmap time which is already hold a mmap_sem as down_write so binder_update_page_range doesn't need to hold a mmap_sem as down_write.

Quoted 6 lines

Quoted 2 lines

Android suffers from mmap_sem contention so let's reduce mmap_sem down_write.

Hi, Minchan:

It seems there is performance regression of this patch.

You mean "This patch aims for solving performance regression" not "This patch makes performance regression"?

Quoted 2 lines

Do you have some test result of android app launch time or binderThroughput?

Quoted 8 lines

Quoted 3 lines

Unfortunately, I don't have any number. The goal is to reduce the number of call mmap_sem as write-side lock because it makes priority inversion of threads easily and that's one of clear part I spot that we don't need write-side lock.

Please always run the binderThroughput tests when making binder changes (there is a binder test suite in the CTS Android tests), as that ensures that you are not causing performance regressions as well as just normal bug regressions :)

Thanks for the information. I didn't notice that such kinds of tests for binder. I will keep it in mind.

Today, I have setup the testing for my phone and found testing was very fluctuating even without my patch. It might be not good with my test skill. I emulated user's behavior with various touch event. With it, I open various apps and play with them several times. Before starting the test, I did "adb shell stop && adb shell start && echo 3 > /proc/sys/vm/drop_caches"

Such 15% noise was very easy to make it.

Ganesh, How did you measure? What's the stddev?

Please let me know how you measure without noise so I'd like to reproduce the result in my phone.

I will do binderThroghput test, too.

2018-04-02 14:34 GMT+08:00 Minchan Kim <[email protected]>:

Quoted 45 lines · Greg Kroah-Hartman

Quoted 26 lines · Minchan Kim

Quoted 25 lines

Hi Ganesh,

Quoted 9 lines · Ganesh Mahendran

2018-03-29 14:54 GMT+08:00 Minchan Kim <[email protected]>:

Quoted 8 lines

binder_update_page_range needs down_write of mmap_sem because vm_insert_page need to change vma->vm_flags to VM_MIXEDMAP unless it is set. However, when I profile binder working, it seems every binder buffers should be mapped in advance by binder_mmap. It means we could set VM_MIXEDMAP in binder_mmap time which is already hold a mmap_sem as down_write so binder_update_page_range doesn't need to hold a mmap_sem as down_write.

Quoted 6 lines

Quoted 2 lines

Android suffers from mmap_sem contention so let's reduce mmap_sem down_write.

Hi, Minchan:

It seems there is performance regression of this patch.

You mean "This patch aims for solving performance regression" not "This patch makes performance regression"?

Quoted 2 lines

Do you have some test result of android app launch time or binderThroughput?

Quoted 8 lines

Quoted 3 lines

Unfortunately, I don't have any number. The goal is to reduce the number of call mmap_sem as write-side lock because it makes priority inversion of threads easily and that's one of clear part I spot that we don't need write-side lock.

Please always run the binderThroughput tests when making binder changes (there is a binder test suite in the CTS Android tests), as that ensures that you are not causing performance regressions as well as just normal bug regressions :)

Thanks for the information. I didn't notice that such kinds of tests for binder. I will keep it in mind.

Today, I have setup the testing for my phone and found testing was very fluctuating even without my patch. It might be not good with my test skill. I emulated user's behavior with various touch event. With it, I open various apps and play with them several times. Before starting the test, I did "adb shell stop && adb shell start && echo 3 > /proc/sys/vm/drop_caches"

Quoted 3 lines

Such 15% noise was very easy to make it.

Ganesh, How did you measure? What's the stddev?

Hi, Minchan:

Sorry for the late response, a little busy these days. :)

We have our own test tools to measure app launch time, or you can use android systrace to get the app launch time. We tested your V1 patch:
https://patchwork.kernel.org/patch/10312057/
and found app lunch time regression.

I will use binderThroghput tool to test the patch today or tomorrow.

Quoted 52 lines

2018-04-02 14:34 GMT+08:00 Minchan Kim <[email protected]>:

Quoted 45 lines · Greg Kroah-Hartman

Quoted 26 lines · Minchan Kim

Quoted 25 lines

Hi Ganesh,

Quoted 9 lines · Ganesh Mahendran

2018-03-29 14:54 GMT+08:00 Minchan Kim <[email protected]>:

Quoted 8 lines

binder_update_page_range needs down_write of mmap_sem because vm_insert_page need to change vma->vm_flags to VM_MIXEDMAP unless it is set. However, when I profile binder working, it seems every binder buffers should be mapped in advance by binder_mmap. It means we could set VM_MIXEDMAP in binder_mmap time which is already hold a mmap_sem as down_write so binder_update_page_range doesn't need to hold a mmap_sem as down_write.

Quoted 6 lines

Quoted 2 lines

Android suffers from mmap_sem contention so let's reduce mmap_sem down_write.

Hi, Minchan:

It seems there is performance regression of this patch.

You mean "This patch aims for solving performance regression" not "This patch makes performance regression"?

Quoted 2 lines

Do you have some test result of android app launch time or binderThroughput?

Quoted 8 lines

Quoted 3 lines

Unfortunately, I don't have any number. The goal is to reduce the number of call mmap_sem as write-side lock because it makes priority inversion of threads easily and that's one of clear part I spot that we don't need write-side lock.

Please always run the binderThroughput tests when making binder changes (there is a binder test suite in the CTS Android tests), as that ensures that you are not causing performance regressions as well as just normal bug regressions :)

Thanks for the information. I didn't notice that such kinds of tests for binder. I will keep it in mind.

Today, I have setup the testing for my phone and found testing was very fluctuating even without my patch. It might be not good with my test skill. I emulated user's behavior with various touch event. With it, I open various apps and play with them several times. Before starting the test, I did "adb shell stop && adb shell start && echo 3 > /proc/sys/vm/drop_caches"

Quoted 3 lines

Such 15% noise was very easy to make it.

Ganesh, How did you measure? What's the stddev?

Hi, Minchan:

Quoted 6 lines

Sorry for the late response, a little busy these days. :)

We have our own test tools to measure app launch time, or you can use android systrace to get the app launch time. We tested your V1 patch:
https://patchwork.kernel.org/patch/10312057/
and found app lunch time regression.

V1 had a bug with VM_MAYWRITE. Could you confirm it with v5?

Please tell me more detail. What apps are slower compared to old? Every apps are slowed with avg 15%? Then, what's the stddev?

The reason I'm asking is as I mentioned, it would be caused by rw_semaphore implementation and priority of threads which calls binder operation so I guess it would be not deterministic.

When I had an simple experiment, it was very fluctuating as I expected. (the testing enviroment might be not good in my side).
If it's real problem on real practice, better fix is not using write_lock of mmap_sem(it's abusing the write-side lock) but should adjust priority, I think. What do you think?

Anyway, before the further discussion, we should confirm the root cause.

Quoted 3 lines

I will use binderThroghput tool to test the patch today or tomorrow.

Thanks. I will do.

2018-04-02 15:11 GMT+08:00 Minchan Kim <[email protected]>:

Quoted 60 lines · Ganesh Mahendran

Quoted 58 lines

2018-04-02 14:34 GMT+08:00 Minchan Kim <[email protected]>:

Quoted 45 lines · Greg Kroah-Hartman

Quoted 26 lines · Minchan Kim

Quoted 25 lines

Hi Ganesh,

Quoted 9 lines · Ganesh Mahendran

2018-03-29 14:54 GMT+08:00 Minchan Kim <[email protected]>:

Quoted 8 lines

binder_update_page_range needs down_write of mmap_sem because vm_insert_page need to change vma->vm_flags to VM_MIXEDMAP unless it is set. However, when I profile binder working, it seems every binder buffers should be mapped in advance by binder_mmap. It means we could set VM_MIXEDMAP in binder_mmap time which is already hold a mmap_sem as down_write so binder_update_page_range doesn't need to hold a mmap_sem as down_write.

Quoted 6 lines

Quoted 2 lines

Android suffers from mmap_sem contention so let's reduce mmap_sem down_write.

Hi, Minchan:

It seems there is performance regression of this patch.

You mean "This patch aims for solving performance regression" not "This patch makes performance regression"?

Quoted 2 lines

Do you have some test result of android app launch time or binderThroughput?

Quoted 8 lines

Quoted 3 lines

Unfortunately, I don't have any number. The goal is to reduce the number of call mmap_sem as write-side lock because it makes priority inversion of threads easily and that's one of clear part I spot that we don't need write-side lock.

Please always run the binderThroughput tests when making binder changes (there is a binder test suite in the CTS Android tests), as that ensures that you are not causing performance regressions as well as just normal bug regressions :)

Thanks for the information. I didn't notice that such kinds of tests for binder. I will keep it in mind.

Today, I have setup the testing for my phone and found testing was very fluctuating even without my patch. It might be not good with my test skill. I emulated user's behavior with various touch event. With it, I open various apps and play with them several times. Before starting the test, I did "adb shell stop && adb shell start && echo 3 > /proc/sys/vm/drop_caches"

Quoted 3 lines

Such 15% noise was very easy to make it.

Ganesh, How did you measure? What's the stddev?

Hi, Minchan:

Sorry for the late response, a little busy these days. :)

We have our own test tools to measure app launch time, or you can use android systrace to get the app launch time. We tested your V1 patch:
https://patchwork.kernel.org/patch/10312057/
and found app lunch time regression.

Quoted 1 line

V1 had a bug with VM_MAYWRITE. Could you confirm it with v5?

I have finished binder Throughput test. The test result is stable, there is no performance
regression found both in v1 and v5.

base patch_v1 patch_v5
-----------------------------------------------------------

            91223.4      90560.2           89644.5
            90520.3      89583.1           89048.2
            89833.2      90247.6           90091.3
            90740.2      90276.7           90994.2
            89703.5      90112.4           89994.6
            89945.1      89122.8           88937.7
            89872.8      90357.3           89307.4
            89913.2      90355.4           89563.8
            88979         90393.4           90182.8
            89577.3      90946.8           90441.4

AVG 90030.8 90195.57 89820.59

Before the test, I stop the android framework by:
adb shell stop

Quoted 3 lines

Please tell me more detail. What apps are slower compared to old? Every apps are slowed with avg 15%? Then, what's the stddev?

Not all of the apps slowed 15%, The app *avg* launch time slowed 15%. And We will re-launch the test tomorrow: base, v1,v5. We will get the test result in two days later. Then I will post all the app launch time details.

Quoted 11 lines

The reason I'm asking is as I mentioned, it would be caused by rw_semaphore implementation and priority of threads which calls binder operation so I guess it would be not deterministic.

When I had an simple experiment, it was very fluctuating as I expected. (the testing enviroment might be not good in my side).
If it's real problem on real practice, better fix is not using write_lock of mmap_sem(it's abusing the write-side lock) but should adjust priority, I think. What do you think?

If you want to narrow the range of the problem. We can disable binder priority inherit, and do not set the priority(currently it is nice -10 or fifo) of top app in Android AMS.
I think we need to wait for the test result to see whether it really has performance
regression.

Hi Ganesh,

Quoted 61 lines · Ganesh Mahendran

2018-04-02 15:11 GMT+08:00 Minchan Kim <[email protected]>:

Quoted 60 lines · Ganesh Mahendran

Quoted 58 lines

2018-04-02 14:34 GMT+08:00 Minchan Kim <[email protected]>:

Quoted 45 lines · Greg Kroah-Hartman

Quoted 26 lines · Minchan Kim

Quoted 25 lines

Hi Ganesh,

Quoted 9 lines · Ganesh Mahendran

2018-03-29 14:54 GMT+08:00 Minchan Kim <[email protected]>:

Quoted 8 lines

binder_update_page_range needs down_write of mmap_sem because vm_insert_page need to change vma->vm_flags to VM_MIXEDMAP unless it is set. However, when I profile binder working, it seems every binder buffers should be mapped in advance by binder_mmap. It means we could set VM_MIXEDMAP in binder_mmap time which is already hold a mmap_sem as down_write so binder_update_page_range doesn't need to hold a mmap_sem as down_write.

Quoted 6 lines

Quoted 2 lines

Android suffers from mmap_sem contention so let's reduce mmap_sem down_write.

Hi, Minchan:

It seems there is performance regression of this patch.

You mean "This patch aims for solving performance regression" not "This patch makes performance regression"?

Quoted 2 lines

Do you have some test result of android app launch time or binderThroughput?

Quoted 8 lines

Quoted 3 lines

Unfortunately, I don't have any number. The goal is to reduce the number of call mmap_sem as write-side lock because it makes priority inversion of threads easily and that's one of clear part I spot that we don't need write-side lock.

Please always run the binderThroughput tests when making binder changes (there is a binder test suite in the CTS Android tests), as that ensures that you are not causing performance regressions as well as just normal bug regressions :)

Thanks for the information. I didn't notice that such kinds of tests for binder. I will keep it in mind.

Today, I have setup the testing for my phone and found testing was very fluctuating even without my patch. It might be not good with my test skill. I emulated user's behavior with various touch event. With it, I open various apps and play with them several times. Before starting the test, I did "adb shell stop && adb shell start && echo 3 > /proc/sys/vm/drop_caches"

Quoted 3 lines

Such 15% noise was very easy to make it.

Ganesh, How did you measure? What's the stddev?

Hi, Minchan:

Sorry for the late response, a little busy these days. :)

We have our own test tools to measure app launch time, or you can use android systrace to get the app launch time. We tested your V1 patch:
https://patchwork.kernel.org/patch/10312057/
and found app lunch time regression.

Quoted 5 lines

Quoted 1 line

V1 had a bug with VM_MAYWRITE. Could you confirm it with v5?

I have finished binder Throughput test. The test result is stable, there is no performance
regression found both in v1 and v5.

Thanks for the test! Now I'm struggling with setting up BinderThrough test. Binder matainers:
If it's really one every binder contributors should do before the sending their patch, couldn't we have them in kernel directory like kselftest? Like me who understand just a part of code, it's hard to download android userspace full code and build/test.

Quoted 13 lines

base patch_v1 patch_v5
-----------------------------------------------------------

            91223.4      90560.2           89644.5
            90520.3      89583.1           89048.2
            89833.2      90247.6           90091.3
            90740.2      90276.7           90994.2
            89703.5      90112.4           89994.6
            89945.1      89122.8           88937.7
            89872.8      90357.3           89307.4
            89913.2      90355.4           89563.8
            88979         90393.4           90182.8
            89577.3      90946.8           90441.4

Quoted 1 line

AVG 90030.8 90195.57 89820.59

Yes, no regression.

Quoted 11 lines

Before the test, I stop the android framework by:
adb shell stop

Quoted 3 lines

Please tell me more detail. What apps are slower compared to old? Every apps are slowed with avg 15%? Then, what's the stddev?

Not all of the apps slowed 15%, The app *avg* launch time slowed 15%. And We will re-launch the test tomorrow: base, v1,v5. We will get the test result in two days later. Then I will post all the app launch time details.

I'm also trying to make stable result in my side but it's really hard to get. Please post stddev of each app as well as avg when you finished testing. I really appreicate you.

Quoted 12 lines

Quoted 11 lines

The reason I'm asking is as I mentioned, it would be caused by rw_semaphore implementation and priority of threads which calls binder operation so I guess it would be not deterministic.

When I had an simple experiment, it was very fluctuating as I expected. (the testing enviroment might be not good in my side).
If it's real problem on real practice, better fix is not using write_lock of mmap_sem(it's abusing the write-side lock) but should adjust priority, I think. What do you think?

Quoted 6 lines

If you want to narrow the range of the problem. We can disable binder priority inherit, and do not set the priority(currently it is nice -10 or fifo) of top app in Android AMS.
I think we need to wait for the test result to see whether it really has performance
regression.

Look at up_write.

(Let's assume process B is head of wait list of rw_semaphore, and then C, D, E) If the process B is trying to down_write and previous lock holder A is called up_write, the only B could be waked up so there is no contention to get CPU slice. It's the current as-is but if we changes B to try to down_read instead of down_write, B should be competed with other down_read C,D,E in so the chance would be rare to be scheduled.

It's really (timing|priority of binder and other threads) problem so I don't understand what you said how we could narrow down the problem with disabling binder priority.

Hi Ganesh,

Isn't there any update?

Quoted 139 lines

Hi Ganesh,

Quoted 61 lines · Ganesh Mahendran

2018-04-02 15:11 GMT+08:00 Minchan Kim <[email protected]>:

Quoted 60 lines · Ganesh Mahendran

Quoted 58 lines

2018-04-02 14:34 GMT+08:00 Minchan Kim <[email protected]>:

Quoted 45 lines · Greg Kroah-Hartman

Quoted 26 lines · Minchan Kim

Quoted 25 lines

Hi Ganesh,

Quoted 9 lines · Ganesh Mahendran

2018-03-29 14:54 GMT+08:00 Minchan Kim <[email protected]>:

Quoted 8 lines

binder_update_page_range needs down_write of mmap_sem because vm_insert_page need to change vma->vm_flags to VM_MIXEDMAP unless it is set. However, when I profile binder working, it seems every binder buffers should be mapped in advance by binder_mmap. It means we could set VM_MIXEDMAP in binder_mmap time which is already hold a mmap_sem as down_write so binder_update_page_range doesn't need to hold a mmap_sem as down_write.

Quoted 6 lines

Quoted 2 lines

Android suffers from mmap_sem contention so let's reduce mmap_sem down_write.

Hi, Minchan:

It seems there is performance regression of this patch.

You mean "This patch aims for solving performance regression" not "This patch makes performance regression"?

Quoted 2 lines

Do you have some test result of android app launch time or binderThroughput?

Quoted 8 lines

Quoted 3 lines

Unfortunately, I don't have any number. The goal is to reduce the number of call mmap_sem as write-side lock because it makes priority inversion of threads easily and that's one of clear part I spot that we don't need write-side lock.

Please always run the binderThroughput tests when making binder changes (there is a binder test suite in the CTS Android tests), as that ensures that you are not causing performance regressions as well as just normal bug regressions :)

Thanks for the information. I didn't notice that such kinds of tests for binder. I will keep it in mind.

Today, I have setup the testing for my phone and found testing was very fluctuating even without my patch. It might be not good with my test skill. I emulated user's behavior with various touch event. With it, I open various apps and play with them several times. Before starting the test, I did "adb shell stop && adb shell start && echo 3 > /proc/sys/vm/drop_caches"

Quoted 3 lines

Such 15% noise was very easy to make it.

Ganesh, How did you measure? What's the stddev?

Hi, Minchan:

Sorry for the late response, a little busy these days. :)

We have our own test tools to measure app launch time, or you can use android systrace to get the app launch time. We tested your V1 patch:
https://patchwork.kernel.org/patch/10312057/
and found app lunch time regression.

Quoted 5 lines

Quoted 1 line

V1 had a bug with VM_MAYWRITE. Could you confirm it with v5?

I have finished binder Throughput test. The test result is stable, there is no performance
regression found both in v1 and v5.

Thanks for the test! Now I'm struggling with setting up BinderThrough test. Binder matainers:
If it's really one every binder contributors should do before the sending their patch, couldn't we have them in kernel directory like kselftest? Like me who understand just a part of code, it's hard to download android userspace full code and build/test.

Quoted 13 lines

base patch_v1 patch_v5
-----------------------------------------------------------

            91223.4      90560.2           89644.5
            90520.3      89583.1           89048.2
            89833.2      90247.6           90091.3
            90740.2      90276.7           90994.2
            89703.5      90112.4           89994.6
            89945.1      89122.8           88937.7
            89872.8      90357.3           89307.4
            89913.2      90355.4           89563.8
            88979         90393.4           90182.8
            89577.3      90946.8           90441.4

Quoted 1 line

AVG 90030.8 90195.57 89820.59

Yes, no regression.

Quoted 11 lines

Before the test, I stop the android framework by:
adb shell stop

Quoted 3 lines

Please tell me more detail. What apps are slower compared to old? Every apps are slowed with avg 15%? Then, what's the stddev?

Not all of the apps slowed 15%, The app *avg* launch time slowed 15%. And We will re-launch the test tomorrow: base, v1,v5. We will get the test result in two days later. Then I will post all the app launch time details.

I'm also trying to make stable result in my side but it's really hard to get. Please post stddev of each app as well as avg when you finished testing. I really appreicate you.

Quoted 12 lines

Quoted 11 lines

The reason I'm asking is as I mentioned, it would be caused by rw_semaphore implementation and priority of threads which calls binder operation so I guess it would be not deterministic.

When I had an simple experiment, it was very fluctuating as I expected. (the testing enviroment might be not good in my side).
If it's real problem on real practice, better fix is not using write_lock of mmap_sem(it's abusing the write-side lock) but should adjust priority, I think. What do you think?

Quoted 6 lines

If you want to narrow the range of the problem. We can disable binder priority inherit, and do not set the priority(currently it is nice -10 or fifo) of top app in Android AMS.
I think we need to wait for the test result to see whether it really has performance
regression.

Look at up_write.

(Let's assume process B is head of wait list of rw_semaphore, and then C, D, E) If the process B is trying to down_write and previous lock holder A is called up_write, the only B could be waked up so there is no contention to get CPU slice. It's the current as-is but if we changes B to try to down_read instead of down_write, B should be competed with other down_read C,D,E in so the chance would be rare to be scheduled.

Quoted 3 lines

It's really (timing|priority of binder and other threads) problem so I don't understand what you said how we could narrow down the problem with disabling binder priority.

--
Kind regards,
Minchan Kim

Hi Ganesh,

Quoted 63 lines · Ganesh Mahendran

2018-04-02 15:11 GMT+08:00 Minchan Kim <[email protected]>:

Quoted 61 lines · Ganesh Mahendran

Quoted 50 lines

2018-04-02 14:34 GMT+08:00 Minchan Kim <[email protected]>:

Quoted 45 lines · Greg Kroah-Hartman

Quoted 26 lines · Minchan Kim

Quoted 25 lines

Hi Ganesh,

Quoted 9 lines · Ganesh Mahendran

2018-03-29 14:54 GMT+08:00 Minchan Kim <[email protected]>:

Quoted 8 lines

binder_update_page_range needs down_write of mmap_sem because vm_insert_page need to change vma->vm_flags to VM_MIXEDMAP unless it is set. However, when I profile binder working, it seems every binder buffers should be mapped in advance by binder_mmap. It means we could set VM_MIXEDMAP in binder_mmap time which is already hold a mmap_sem as down_write so binder_update_page_range doesn't need to hold a mmap_sem as down_write.

Quoted 6 lines

Quoted 2 lines

Android suffers from mmap_sem contention so let's reduce mmap_sem down_write.

Hi, Minchan:

It seems there is performance regression of this patch.

You mean "This patch aims for solving performance regression" not "This patch makes performance regression"?

Quoted 2 lines

Do you have some test result of android app launch time or binderThroughput?

Quoted 8 lines

Quoted 3 lines

Unfortunately, I don't have any number. The goal is to reduce the number of call mmap_sem as write-side lock because it makes priority inversion of threads easily and that's one of clear part I spot that we don't need write-side lock.

Please always run the binderThroughput tests when making binder changes (there is a binder test suite in the CTS Android tests), as that ensures that you are not causing performance regressions as well as just normal bug regressions :)

Thanks for the information. I didn't notice that such kinds of tests for binder. I will keep it in mind.

Today, I have setup the testing for my phone and found testing was very fluctuating even without my patch. It might be not good with my test skill. I emulated user's behavior with various touch event. With it, I open various apps and play with them several times. Before starting the test, I did "adb shell stop && adb shell start && echo 3 > /proc/sys/vm/drop_caches"

Quoted 3 lines

Such 15% noise was very easy to make it.

Ganesh, How did you measure? What's the stddev?

Quoted 8 lines

Hi, Minchan:

Sorry for the late response, a little busy these days. :)

We have our own test tools to measure app launch time, or you can use android systrace to get the app launch time. We tested your V1 patch:
https://patchwork.kernel.org/patch/10312057/
and found app lunch time regression.

V1 had a bug with VM_MAYWRITE. Could you confirm it with v5?

Quoted 3 lines

I have finished binder Throughput test. The test result is stable, there is no performance
regression found both in v1 and v5.

Thanks for the test! Now I'm struggling with setting up BinderThrough test. Binder matainers:
If it's really one every binder contributors should do before the sending their patch, couldn't we have them in kernel directory like kselftest?

Like me who understand just a part of code, it's hard to download android userspace full code and build/test.

Quoted 13 lines

base patch_v1 patch_v5
-----------------------------------------------------------

            91223.4      90560.2           89644.5
            90520.3      89583.1           89048.2
            89833.2      90247.6           90091.3
            90740.2      90276.7           90994.2
            89703.5      90112.4           89994.6
            89945.1      89122.8           88937.7
            89872.8      90357.3           89307.4
            89913.2      90355.4           89563.8
            88979         90393.4           90182.8
            89577.3      90946.8           90441.4

Quoted 1 line

AVG 90030.8 90195.57 89820.59

Yes, no regression.

Quoted 8 lines

Before the test, I stop the android framework by:
adb shell stop

Quoted 3 lines

Please tell me more detail. What apps are slower compared to old? Every apps are slowed with avg 15%? Then, what's the stddev?

Quoted 3 lines

Not all of the apps slowed 15%, The app *avg* launch time slowed 15%. And We will re-launch the test tomorrow: base, v1,v5. We will get the test result in two days later. Then I will post all the app launch time details.

I'm also trying to make stable result in my side but it's really hard to get. Please post stddev of each app as well as avg when you finished testing. I really appreicate you.

Quoted 13 lines

Quoted 7 lines

The reason I'm asking is as I mentioned, it would be caused by rw_semaphore implementation and priority of threads which calls binder operation so I guess it would be not deterministic.

Quoted 5 lines

When I had an simple experiment, it was very fluctuating as I expected. (the testing enviroment might be not good in my side).
If it's real problem on real practice, better fix is not using write_lock of mmap_sem(it's abusing the write-side lock) but should adjust priority, I think. What do you think?

Quoted 6 lines

If you want to narrow the range of the problem. We can disable binder priority inherit, and do not set the priority(currently it is nice -10 or fifo) of top app in Android AMS.
I think we need to wait for the test result to see whether it really has performance
regression.

Look at up_write.

(Let's assume process B is head of wait list of rw_semaphore, and then C, D, E) If the process B is trying to down_write and previous lock holder A is called up_write, the only B could be waked up so there is no contention to get CPU slice. It's the current as-is but if we changes B to try to down_read instead of down_write, B should be competed with other down_read C,D,E in so the chance would be rare to be scheduled.

It's really (timing|priority of binder and other threads) problem so I don't understand what you said how we could narrow down the problem with disabling binder priority.

Quoted 28 lines

Quoted 12 lines

base patch_v1 patch_v5
-----------------------------------------------------------

            91223.4      90560.2           89644.5
            90520.3      89583.1           89048.2
            89833.2      90247.6           90091.3
            90740.2      90276.7           90994.2
            89703.5      90112.4           89994.6
            89945.1      89122.8           88937.7
            89872.8      90357.3           89307.4
            89913.2      90355.4           89563.8
            88979         90393.4           90182.8
            89577.3      90946.8           90441.4

Quoted 1 line

AVG 90030.8 90195.57 89820.59

Yes, no regression.

Quoted 11 lines

Before the test, I stop the android framework by:
adb shell stop

Quoted 3 lines

Please tell me more detail. What apps are slower compared to old? Every apps are slowed with avg 15%? Then, what's the stddev?

Not all of the apps slowed 15%, The app *avg* launch time slowed 15%. And We will re-launch the test tomorrow: base, v1,v5. We will get the test result in two days later. Then I will post all the app launch time details.

Quoted 3 lines

I'm also trying to make stable result in my side but it's really hard to get. Please post stddev of each app as well as avg when you finished testing. I really appreicate you.

What do you mean by stddev?

We test 80 loops and launch ~40 apps in each loop. Below is the app launch time result:

app base v1 diff percent v5 diff percent
----
com.tencent.mobileqq 829 834 5 1% 879 50 6% com.tencent.qqmusic 799 790 -9 -1% 764 -35 -4% com.tencent.mtt 659 655 -4 -1% 979 320 49% com.UCMobile 1149 1144 -5 0% 927 -222 -19% com.qiyi.video 1557 1579 22 1% 1497 -60 -4% com.baidu.BaiduMap 1137 1136 -1 0% 1096 -41 -4% tv.danmaku.bili 3642 3655 13 0% 3538 -104 -3% com.sdu.didi.psnger 4334 4352 18 0% 4224 -110 -3% com.ss.android.ugc.aweme 1958 1970 12 1% 1884 -74 -4% air.tv.douyu.android 3333 3371 38 1% 3251 -82 -2% me.ele 3183 3182 -1 0% 3178 -5 0% com.autonavi.minimap 1920 1922 2 0% 1868 -52 -3% com.duowan.kiwi 1452 1457 5 0% 1349 -103 -7% com.v.study 3549 3558 9 0% 3519 -30 -1% com.qqgame.hlddz 4074 4060 -14 0% 4443 369 9% com.ss.android.article.news 1631 1680 49 3% 1649 18 1% com.jingdong.app.mall 1448 1443 -5 0% 1323 -125 -9% com.tencent.tmgp.pubgmhd 1703 1706 3 0% 1601 -102 -6% com.kugou.android 854 862 8 1% 791 -63 -7% com.kuaikan.comic 1341 1374 33 2% 2118 777 58% com.smile.gifmaker 798 686 -112 -14% 642 -156 -20% com.hunantv.imgo.activity 1560 1616 56 4% 1569 9 1% com.mt.mtxx.mtxx 1746 1838 92 5% 1773 27 2% com.sankuai.meituan 3610 3697 87 2% 3551 -59 -2% com.sankuai.meituan.takeoutnew 3376 3387 11 0% 3325 -51 -2% com.meitu.meiyancamera 1905 2010 105 6% 1870 -35 -2% com.tencent.karaoke 888 906 18 2% 896 8 1% com.taobao.taobao 3344 3406 62 2% 3368 24 1% com.tencent.qqlive 1314 1345 31 2% 1499 185 14% com.tmall.wireless 3746 3735 -11 0% 3699 -47 -1% com.tencent.tmgp.sgame 3250 3513 263 8% 3707 457 14% com.netease.cloudmusic 2550 2570 20 1% 2546 -4 0% com.sina.weibo 2201 2240 39 2% 2191 -10 0% com.tencent.mm 638 645 7 1% 690 52 8% com.immomo.momo 1536 1554 18 1% 1563 27 2% com.xiaomi.hm.health 915 926 11 1% 888 -27 -3% com.youku.phone 1881 1820 -61 -3% 1880 -1 0% com.eg.android.AlipayGphone 1536 1557 21 1% 1624 88 6% com.meituan.qcs.c.android 3140 3533 393 13% 3171 31 1%
-----
average 2064 2095 31 1.50% 2085 21 1%

1% is in the fluctuating range of our tool. So no obvious regression found in app launch time.

Hi Ganesh,

On Tue, Apr 10, 2018 at 04:52:58PM +0800, Ganesh Mahendran wrote:

< snip >

Quoted 28 lines

Quoted 28 lines

Quoted 12 lines

base patch_v1 patch_v5
-----------------------------------------------------------

            91223.4      90560.2           89644.5
            90520.3      89583.1           89048.2
            89833.2      90247.6           90091.3
            90740.2      90276.7           90994.2
            89703.5      90112.4           89994.6
            89945.1      89122.8           88937.7
            89872.8      90357.3           89307.4
            89913.2      90355.4           89563.8
            88979         90393.4           90182.8
            89577.3      90946.8           90441.4

Quoted 1 line

AVG 90030.8 90195.57 89820.59

Yes, no regression.

Quoted 11 lines

Before the test, I stop the android framework by:
adb shell stop

Quoted 3 lines

Please tell me more detail. What apps are slower compared to old? Every apps are slowed with avg 15%? Then, what's the stddev?

Not all of the apps slowed 15%, The app *avg* launch time slowed 15%. And We will re-launch the test tomorrow: base, v1,v5. We will get the test result in two days later. Then I will post all the app launch time details.

Quoted 5 lines

Quoted 3 lines

I'm also trying to make stable result in my side but it's really hard to get. Please post stddev of each app as well as avg when you finished testing. I really appreicate you.

What do you mean by stddev?

Standard deviation.

Quoted 48 lines

We test 80 loops and launch ~40 apps in each loop. Below is the app launch time result:

app base v1 diff percent v5 diff percent
----
com.tencent.mobileqq 829 834 5 1% 879 50 6% com.tencent.qqmusic 799 790 -9 -1% 764 -35 -4% com.tencent.mtt 659 655 -4 -1% 979 320 49% com.UCMobile 1149 1144 -5 0% 927 -222 -19% com.qiyi.video 1557 1579 22 1% 1497 -60 -4% com.baidu.BaiduMap 1137 1136 -1 0% 1096 -41 -4% tv.danmaku.bili 3642 3655 13 0% 3538 -104 -3% com.sdu.didi.psnger 4334 4352 18 0% 4224 -110 -3% com.ss.android.ugc.aweme 1958 1970 12 1% 1884 -74 -4% air.tv.douyu.android 3333 3371 38 1% 3251 -82 -2% me.ele 3183 3182 -1 0% 3178 -5 0% com.autonavi.minimap 1920 1922 2 0% 1868 -52 -3% com.duowan.kiwi 1452 1457 5 0% 1349 -103 -7% com.v.study 3549 3558 9 0% 3519 -30 -1% com.qqgame.hlddz 4074 4060 -14 0% 4443 369 9% com.ss.android.article.news 1631 1680 49 3% 1649 18 1% com.jingdong.app.mall 1448 1443 -5 0% 1323 -125 -9% com.tencent.tmgp.pubgmhd 1703 1706 3 0% 1601 -102 -6% com.kugou.android 854 862 8 1% 791 -63 -7% com.kuaikan.comic 1341 1374 33 2% 2118 777 58% com.smile.gifmaker 798 686 -112 -14% 642 -156 -20% com.hunantv.imgo.activity 1560 1616 56 4% 1569 9 1% com.mt.mtxx.mtxx 1746 1838 92 5% 1773 27 2% com.sankuai.meituan 3610 3697 87 2% 3551 -59 -2% com.sankuai.meituan.takeoutnew 3376 3387 11 0% 3325 -51 -2% com.meitu.meiyancamera 1905 2010 105 6% 1870 -35 -2% com.tencent.karaoke 888 906 18 2% 896 8 1% com.taobao.taobao 3344 3406 62 2% 3368 24 1% com.tencent.qqlive 1314 1345 31 2% 1499 185 14% com.tmall.wireless 3746 3735 -11 0% 3699 -47 -1% com.tencent.tmgp.sgame 3250 3513 263 8% 3707 457 14% com.netease.cloudmusic 2550 2570 20 1% 2546 -4 0% com.sina.weibo 2201 2240 39 2% 2191 -10 0% com.tencent.mm 638 645 7 1% 690 52 8% com.immomo.momo 1536 1554 18 1% 1563 27 2% com.xiaomi.hm.health 915 926 11 1% 888 -27 -3% com.youku.phone 1881 1820 -61 -3% 1880 -1 0% com.eg.android.AlipayGphone 1536 1557 21 1% 1624 88 6% com.meituan.qcs.c.android 3140 3533 393 13% 3171 31 1%
-----
average 2064 2095 31 1.50% 2085 21 1%

Quoted 2 lines

1% is in the fluctuating range of our tool. So no obvious regression found in app launch time.

Thanks.

Greg, Ganesh finally confirmed there is no regression. Could you pick up the patch?

Quoted 89 lines · Minchan Kim

Hi Ganesh,

On Tue, Apr 10, 2018 at 04:52:58PM +0800, Ganesh Mahendran wrote:

< snip >

Quoted 28 lines

Quoted 28 lines

Quoted 12 lines

base patch_v1 patch_v5
-----------------------------------------------------------

            91223.4      90560.2           89644.5
            90520.3      89583.1           89048.2
            89833.2      90247.6           90091.3
            90740.2      90276.7           90994.2
            89703.5      90112.4           89994.6
            89945.1      89122.8           88937.7
            89872.8      90357.3           89307.4
            89913.2      90355.4           89563.8
            88979         90393.4           90182.8
            89577.3      90946.8           90441.4

Quoted 1 line

AVG 90030.8 90195.57 89820.59

Yes, no regression.

Quoted 11 lines

Before the test, I stop the android framework by:
adb shell stop

Quoted 3 lines

Please tell me more detail. What apps are slower compared to old? Every apps are slowed with avg 15%? Then, what's the stddev?

Not all of the apps slowed 15%, The app *avg* launch time slowed 15%. And We will re-launch the test tomorrow: base, v1,v5. We will get the test result in two days later. Then I will post all the app launch time details.

Quoted 5 lines

Quoted 3 lines

I'm also trying to make stable result in my side but it's really hard to get. Please post stddev of each app as well as avg when you finished testing. I really appreicate you.

What do you mean by stddev?

Standard deviation.

Quoted 48 lines

We test 80 loops and launch ~40 apps in each loop. Below is the app launch time result:

app base v1 diff percent v5 diff percent
----
com.tencent.mobileqq 829 834 5 1% 879 50 6% com.tencent.qqmusic 799 790 -9 -1% 764 -35 -4% com.tencent.mtt 659 655 -4 -1% 979 320 49% com.UCMobile 1149 1144 -5 0% 927 -222 -19% com.qiyi.video 1557 1579 22 1% 1497 -60 -4% com.baidu.BaiduMap 1137 1136 -1 0% 1096 -41 -4% tv.danmaku.bili 3642 3655 13 0% 3538 -104 -3% com.sdu.didi.psnger 4334 4352 18 0% 4224 -110 -3% com.ss.android.ugc.aweme 1958 1970 12 1% 1884 -74 -4% air.tv.douyu.android 3333 3371 38 1% 3251 -82 -2% me.ele 3183 3182 -1 0% 3178 -5 0% com.autonavi.minimap 1920 1922 2 0% 1868 -52 -3% com.duowan.kiwi 1452 1457 5 0% 1349 -103 -7% com.v.study 3549 3558 9 0% 3519 -30 -1% com.qqgame.hlddz 4074 4060 -14 0% 4443 369 9% com.ss.android.article.news 1631 1680 49 3% 1649 18 1% com.jingdong.app.mall 1448 1443 -5 0% 1323 -125 -9% com.tencent.tmgp.pubgmhd 1703 1706 3 0% 1601 -102 -6% com.kugou.android 854 862 8 1% 791 -63 -7% com.kuaikan.comic 1341 1374 33 2% 2118 777 58% com.smile.gifmaker 798 686 -112 -14% 642 -156 -20% com.hunantv.imgo.activity 1560 1616 56 4% 1569 9 1% com.mt.mtxx.mtxx 1746 1838 92 5% 1773 27 2% com.sankuai.meituan 3610 3697 87 2% 3551 -59 -2% com.sankuai.meituan.takeoutnew 3376 3387 11 0% 3325 -51 -2% com.meitu.meiyancamera 1905 2010 105 6% 1870 -35 -2% com.tencent.karaoke 888 906 18 2% 896 8 1% com.taobao.taobao 3344 3406 62 2% 3368 24 1% com.tencent.qqlive 1314 1345 31 2% 1499 185 14% com.tmall.wireless 3746 3735 -11 0% 3699 -47 -1% com.tencent.tmgp.sgame 3250 3513 263 8% 3707 457 14% com.netease.cloudmusic 2550 2570 20 1% 2546 -4 0% com.sina.weibo 2201 2240 39 2% 2191 -10 0% com.tencent.mm 638 645 7 1% 690 52 8% com.immomo.momo 1536 1554 18 1% 1563 27 2% com.xiaomi.hm.health 915 926 11 1% 888 -27 -3% com.youku.phone 1881 1820 -61 -3% 1880 -1 0% com.eg.android.AlipayGphone 1536 1557 21 1% 1624 88 6% com.meituan.qcs.c.android 3140 3533 393 13% 3171 31 1%
-----
average 2064 2095 31 1.50% 2085 21 1%

Quoted 7 lines

Quoted 2 lines

1% is in the fluctuating range of our tool. So no obvious regression found in app launch time.

Thanks.

Greg, Ganesh finally confirmed there is no regression. Could you pick up the patch?

If there is no regression, and no speed up either, why is it needed? :)

And is app launch time really the best binder benchmark? What about the throughput and latency tests? Do those show any changes? I'm kind of lost as to what is really happening here, sorry, the merge window is not the time for me to be keeping track of this type of thing...

Quoted 92 lines · Minchan Kim

Hi Ganesh,

On Tue, Apr 10, 2018 at 04:52:58PM +0800, Ganesh Mahendran wrote:

< snip >

Quoted 28 lines

Quoted 28 lines

Quoted 12 lines

base patch_v1 patch_v5
-----------------------------------------------------------

            91223.4      90560.2           89644.5
            90520.3      89583.1           89048.2
            89833.2      90247.6           90091.3
            90740.2      90276.7           90994.2
            89703.5      90112.4           89994.6
            89945.1      89122.8           88937.7
            89872.8      90357.3           89307.4
            89913.2      90355.4           89563.8
            88979         90393.4           90182.8
            89577.3      90946.8           90441.4

Quoted 1 line

AVG 90030.8 90195.57 89820.59

Yes, no regression.

Quoted 11 lines

Before the test, I stop the android framework by:
adb shell stop

Quoted 3 lines

Please tell me more detail. What apps are slower compared to old? Every apps are slowed with avg 15%? Then, what's the stddev?

Not all of the apps slowed 15%, The app *avg* launch time slowed 15%. And We will re-launch the test tomorrow: base, v1,v5. We will get the test result in two days later. Then I will post all the app launch time details.

Quoted 5 lines

Quoted 3 lines

I'm also trying to make stable result in my side but it's really hard to get. Please post stddev of each app as well as avg when you finished testing. I really appreicate you.

What do you mean by stddev?

Standard deviation.

Quoted 50 lines

We test 80 loops and launch ~40 apps in each loop. Below is the app launch time result:

app base v1 diff percent v5 diff percent
----
com.tencent.mobileqq 829 834 5 1% 879 50 6% com.tencent.qqmusic 799 790 -9 -1% 764 -35 -4% com.tencent.mtt 659 655 -4 -1% 979 320 49% com.UCMobile 1149 1144 -5 0% 927 -222 -19% com.qiyi.video 1557 1579 22 1% 1497 -60 -4% com.baidu.BaiduMap 1137 1136 -1 0% 1096 -41 -4% tv.danmaku.bili 3642 3655 13 0% 3538 -104 -3% com.sdu.didi.psnger 4334 4352 18 0% 4224 -110 -3% com.ss.android.ugc.aweme 1958 1970 12 1% 1884 -74 -4% air.tv.douyu.android 3333 3371 38 1% 3251 -82 -2% me.ele 3183 3182 -1 0% 3178 -5 0% com.autonavi.minimap 1920 1922 2 0% 1868 -52 -3% com.duowan.kiwi 1452 1457 5 0% 1349 -103 -7% com.v.study 3549 3558 9 0% 3519 -30 -1% com.qqgame.hlddz 4074 4060 -14 0% 4443 369 9% com.ss.android.article.news 1631 1680 49 3% 1649 18 1% com.jingdong.app.mall 1448 1443 -5 0% 1323 -125 -9% com.tencent.tmgp.pubgmhd 1703 1706 3 0% 1601 -102 -6% com.kugou.android 854 862 8 1% 791 -63 -7% com.kuaikan.comic 1341 1374 33 2% 2118 777 58% com.smile.gifmaker 798 686 -112 -14% 642 -156 -20% com.hunantv.imgo.activity 1560 1616 56 4% 1569 9 1% com.mt.mtxx.mtxx 1746 1838 92 5% 1773 27 2% com.sankuai.meituan 3610 3697 87 2% 3551 -59 -2% com.sankuai.meituan.takeoutnew 3376 3387 11 0% 3325 -51 -2% com.meitu.meiyancamera 1905 2010 105 6% 1870 -35 -2% com.tencent.karaoke 888 906 18 2% 896 8 1% com.taobao.taobao 3344 3406 62 2% 3368 24 1% com.tencent.qqlive 1314 1345 31 2% 1499 185 14% com.tmall.wireless 3746 3735 -11 0% 3699 -47 -1% com.tencent.tmgp.sgame 3250 3513 263 8% 3707 457 14% com.netease.cloudmusic 2550 2570 20 1% 2546 -4 0% com.sina.weibo 2201 2240 39 2% 2191 -10 0% com.tencent.mm 638 645 7 1% 690 52 8% com.immomo.momo 1536 1554 18 1% 1563 27 2% com.xiaomi.hm.health 915 926 11 1% 888 -27 -3% com.youku.phone 1881 1820 -61 -3% 1880 -1 0% com.eg.android.AlipayGphone 1536 1557 21 1% 1624 88 6% com.meituan.qcs.c.android 3140 3533 393 13% 3171 31 1%
-----
average 2064 2095 31 1.50% 2085 21 1%

1% is in the fluctuating range of our tool. So no obvious regression found in app launch time.

Quoted 4 lines

Thanks.

Greg, Ganesh finally confirmed there is no regression. Could you pick up the patch?

If there is no regression, and no speed up either, why is it needed? :)

And is app launch time really the best binder benchmark? What about the throughput and latency tests? Do those show any changes? I'm kind of

lost as to what is really happening here, sorry, the merge window is not the time for me to be keeping track of this type of thing...

thanks,

greg k-h