Re: [PATCH v2 0/2] Make reader optimistic spinning optional

[Bongkyu Kim]

On Tue, Apr 02, 2024 at 06:27:40PM -0700, John Stultz wrote:
> On Tue, Apr 2, 2024 at 6:21 PM Bongkyu Kim <bongkyu7.kim@xxxxxxxxxxx> wrote:
> > On Tue, Apr 02, 2024 at 04:46:06PM -0700, John Stultz wrote:
> > > On Thu, Aug 31, 2023 at 6:07 PM Bongkyu Kim <bongkyu7.kim@xxxxxxxxxxx> wrote:
> > > >
> > > > This is rework of the following discussed patch.
> > > > https://lore.kernel.org/all/20230613043308.GA1027@xxxxxxxxxxxxxxxxxxxxxxxxx/
> > > >
> > > > Changes from the previous patch
> > > > - Split to revert and modify patches
> > > > - Change according to Waiman Long's review
> > > >     More wording to documentation part
> > > >     Change module_param to early_param
> > > >     Code change by Waiman Long's suggestion
> > > >
> > > > In mobile environment, reader optimistic spinning is still useful
> > > > because there're not many readers. In my test result at android device,
> > > > it improves application startup time about 3.8%
> > > > App startup time is most important factor for android user expriences.
> > > > So, re-enable reader optimistic spinning by this commit. And,
> > > > make it optional feature by cmdline.
> > > >
> > > > Test result:
> > > > This is 15 application startup performance in our exynos soc.
> > > > - Cortex A78*2 + Cortex A55*6
> > > > - unit: ms (lower is better)
> > > >
> > > > Application             base  opt_rspin  Diff  Diff(%)
> > > > --------------------  ------  ---------  ----  -------
> > > > * Total(geomean)         343        330   -13    +3.8%
> > > > --------------------  ------  ---------  ----  -------
> > > > helloworld               110        108    -2    +1.8%
> > > > Amazon_Seller            397        388    -9    +2.3%
> > > > Whatsapp                 311        304    -7    +2.3%
> > > > Simple_PDF_Reader        500        463   -37    +7.4%
> > > > FaceApp                  330        317   -13    +3.9%
> > > > Timestamp_Camera_Free    451        443    -8    +1.8%
> > > > Kindle                   629        597   -32    +5.1%
> > > > Coinbase                 243        233   -10    +4.1%
> > > > Firefox                  425        399   -26    +6.1%
> > > > Candy_Crush_Soda         552        538   -14    +2.5%
> > > > Hill_Climb_Racing        245        230   -15    +6.1%
> > > > Call_Recorder            437        426   -11    +2.5%
> > > > Color_Fill_3D            190        180   -10    +5.3%
> > > > eToro                    512        505    -7    +1.4%
> > > > GroupMe                  281        266   -15    +5.3%
> > > >
> > >
> > > Hey Bongkyu,
> > >   I wanted to reach out to see what the current status of this patch
> > > set? I'm seeing other parties trying to work around the loss of the
> > > optimistic spinning functionality since commit 617f3ef95177
> > > ("locking/rwsem: Remove reader optimistic spinning") as well, with
> > > their own custom variants (providing some substantial gains), and
> > > would really like to have a common solution.
> > >
> >
> > I didn't get an reply, so I've been waiting.
> > Could you let me know about their patch?
> 
> I don't have insight/access to any other implementations, but I have
> nudged folks to test your patch and chime in here.
> 
> Mostly I just wanted to share that others are also seeing performance
> trouble from the loss of optimistic spinning, so it would be good to
> get some sort of shared solution upstream.
> 
> thanks
> -john
> 

Thanks for your sharing!
Bongkyu.