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


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 -- 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/