I notice that sonarqubebot complains about the goto, and the reviewer
suggests adding "// NOSONAR" to suppress that. I suggest we retain that
behavior (frankly I'm not familiar enough with sonarqubebot to know if we
could globally suppress it if we wanted to...)

I don't object to a (very) limited use of goto in (very) strategic
situations. But IMO it should be unusual enough that forcing the developer
to add a comment to explicitly suppress the gripe is reasonable.

Bill Pirkle
Software Engineer
www.wikimediafoundation.org


On Sun, Aug 1, 2021 at 5:37 AM Tim Starling <[email protected]> wrote:

> On 1/8/21 4:04 pm, rupert THURNER wrote:
>
> you triggered me reading more about it though. the commit comment
> states it takes 30% less instructions:
>   Measuring instruction count per iteration with perf stat, averaged over
>   10M iterations, PS1. Test case:
>   Html::openElement('a', [ 'class' => [ 'foo', 'bar' ] ] )
>
>   * Baseline: 11160.7265433
>   * in_array(): 10390.3837233
>   * dropDefaults() changes: 9674.1248824
>   * expandAttributes() misc: 9248.1947500
>   * implode/explode and space check: 8318.9800417
>   * Sanitizer inline: 8021.7371794
>
> does this mean these changes bring 30% speed improvement? that is
> incredible!
>
> Well, 30% reduction in instruction count. Time reduction is about 25%,
> although you can take the reciprocal of that (1/0.75) and call it 34% speed
> improvement.
>
> I used instruction count rather than time because you can get 4-5
> significant figures of accuracy, i.e. the first 4-5 digits stay the same
> between runs, despite background activity, so you can measure small changes
> very accurately.
>
> how often is this part called to retrieve one article?
>
> Errr... let's just say there's no need to name a second day after me. It's
> a small change.
>
> The broader context is T284274 <https://phabricator.wikimedia.org/T284274>--
> I'm trying to make sure you can view the history page with limit=5000
> without seeing a timeout. My change to Thanks probably cut render time for
> big history pages by 50% -- that's how much the 95th and 99th percentile
> service times dropped by. Html::openElement() is a smaller piece of a
> smaller piece of the puzzle.
>
> -- Tim Starling
> _______________________________________________
> Wikitech-l mailing list -- [email protected]
> To unsubscribe send an email to [email protected]
> https://lists.wikimedia.org/postorius/lists/wikitech-l.lists.wikimedia.org/
_______________________________________________
Wikitech-l mailing list -- [email protected]
To unsubscribe send an email to [email protected]
https://lists.wikimedia.org/postorius/lists/wikitech-l.lists.wikimedia.org/

Reply via email to