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 tstarling@wikimedia.org 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 -- wikitech-l@lists.wikimedia.org To unsubscribe send an email to wikitech-l-leave@lists.wikimedia.org https://lists.wikimedia.org/postorius/lists/wikitech-l.lists.wikimedia.org/