Skip to content

Commit b4f901b

Browse files
palimondopalimondo
committed
[benchmark] Naming Convention
New benchmark naming convention for better readability and improved naming system that accounts for performance coverage growth going forward.
1 parent 84c93c9 commit b4f901b

File tree

3 files changed

+183
-12
lines changed

3 files changed

+183
-12
lines changed

‎benchmark/Naming.md

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
# Naming Convention
2+
3+
Historically, benchmark names in the Swift Benchmark Suite were derived from the
4+
name of the `runFunction`, which by convention started with prefix `run_`,
5+
followed by the benchmark name. Therefore most of the legacy benchmark names
6+
conform to the [`UpperCamelCase`](http://bit.ly/UpperCamelCase) convention.
7+
After introduction of
8+
[`BenchmarkInfo`](https://github.com/apple/swift/pull/12048)
9+
to describe the benchmark metadata, names can be any string. To create more
10+
cohesive and well structured system, names of newly added benchmarks should meet
11+
the following set of requirements:
12+
13+
<ul>
14+
<li>
15+
<!-- The <li> content with <details> is pre-formatted as HTML, to work around
16+
Markdown renderer interrupting the paragraph, which creates an ugly gap. -->
17+
<strong>Letters, numbers and dots.</strong> Start with short unique name in
18+
<code>UpperCamelCase</code>.
19+
For a family of benchmarks, separate the name components with periods.
20+
<details>
21+
22+
Very long compound names using `UpperCamelCase` are hard to read. Use `.` to
23+
increase readability and structure.
24+
25+
Prefer unique and creative name to nondescript generic term, unless the
26+
benchmark is testing individual method on a concrete type.
27+
28+
````
29+
⛔️ Dictionary2
30+
✅ AngryPhonebook
31+
✅ Dictionary.AnyHashable.String.update
32+
✅ Array.append.Array.Int
33+
````
34+
35+
Benchmark names are used to run individual tests when passed as command line
36+
arguments to the benchmark driver. Special characters that could be interpreted
37+
by the shell would require quoting. Stick to ASCII letters, numbers and period.
38+
Exceptionally:
39+
40+
* Use **`-`** only to denote control flow constructs like `for-in` or `if-let`.
41+
* Use **`!`** and **`?`** for optional types, conditional or forced downcasting
42+
and optional chaining.
43+
44+
````
45+
✅ OCB.NSArray.AnyObject.as?.Array.NSString
46+
✅ OCB.NSArray.AnyObject.as!.Array.String
47+
✅ Array.append.Array.Int?
48+
✅ Flatten.Array.Tuple4.for-in.reserved
49+
````
50+
</details><p><!-- spacer --></p></li>
51+
<li>
52+
<strong>Use groups and variants</strong> to structure the benchmark family by
53+
degrees of freedom (e.g. different types implementing a protocol, value vs.
54+
reference types etc.). Use <strong>numbered suffixes</strong> to denote
55+
differently sized variants.
56+
<details>
57+
58+
Benchmarks in a family can be grouped by the tested operation, method or varied
59+
by types and different workload sizes. It might be necessary to abbreviate some names to fit the size limit, based on the longest combination. Choose consistent names for the components throughout all members in the family, to allow for relative comparison across the different axis of variation.
60+
61+
````
62+
✅ Seq.dropFirst.Array
63+
✅ Seq.dropLast.Range.lazy
64+
✅ Seq.dropWhile.UnfoldSeq
65+
✅ Seq.prefix.AnySeq.RangeIter.lazy
66+
✅ Seq.prefixWhile.AnyCol.Array
67+
✅ Seq.suffix.AnySeq.UnfoldSeq.lazy
68+
69+
✅ Existential.Array.ConditionalShift.Ref1
70+
✅ Existential.Array.Mutating.Ref2
71+
✅ Existential.Array.method.1x.Ref3
72+
✅ Existential.Array.method.2x.Ref4
73+
✅ Existential.Array.Shift.Val0
74+
✅ Existential.MutatingAndNonMutating.Val1
75+
✅ Existential.Mutating.Val2
76+
✅ Existential.method.1x.Val3
77+
✅ Existential.method.2x.Val4
78+
✅ Existential.Pass2.method.1x.Ref1
79+
✅ Existential.Pass2.method.2x.Ref2
80+
81+
✅ Set.isSubset.Int25
82+
✅ Set.symmetricDifference.Int50
83+
````
84+
85+
</details><p><!-- spacer --></p></li>
86+
<li>
87+
<strong>Groups and types are <code>UpperCase</code>, methods are
88+
<code>lowerCase</code>.</strong>
89+
<details>
90+
91+
Use periods to separate the name components in variants derived from specialised
92+
generic types or significant method chains.
93+
94+
````
95+
⛔️ InsertCharacterStartIndex
96+
⛔️ InsertCharacterTowardsEndIndexNonASCII
97+
````
98+
99+
There's no need to be literal with type names. **Be descriptive**:
100+
101+
````
102+
✅ Flatten.Array.Tuple4.lazy.flatMap
103+
✅ String.insert.ASCIIChar.StartIndex
104+
✅ String.insert.EmojiChar.NearEnd
105+
````
106+
107+
</details><p><!-- spacer --></p></li>
108+
<li>
109+
<strong>Keep it short.</strong> 40 characters at most. Abbreviate if necessary!
110+
<details>
111+
112+
Benchmarking results are reported on GitHub and very long names are causing
113+
horizontal table scrolling which unfortunately obscures the columns with actual
114+
measurements. Fixed upper size limit also helps with the formatted console
115+
output, when measuring locally. *It is more important for benchmark's name to be
116+
unique and short, than overly descriptive.*
117+
118+
Prefer concise names for potential benchmark family extensions. Leave out the
119+
nested types from variants if they aren't strictly necessary for disambiguation.
120+
If there's potentially valuable future variant, like testing `ContinuousArray`,
121+
keep the `Array` now, allowing for addition of `ContArr` variants later.
122+
123+
Use **`Val`** and **`Ref`** as short descriptors for variants that compare value
124+
types (`struct`, `Int`) with reference types (often named with `Class` in the
125+
legacy-style).
126+
Prefer **`Char`** to `Character`, which can be combined with codepage or
127+
language prefix/suffix when necessary (`ASCIIChar`). For benchmarks that measure
128+
`String`'s Unicode performance for various languages, use
129+
[two letter codes](https://en.wikipedia.org/wiki/ISO_639-1) instead of spelling
130+
out the whole language names.
131+
132+
In a pinch, even C-style naming with two letter prefixes `OC` (for Objective-C)
133+
or abbreviations like `Str` and `Arr` are OK, if it helps to fit a system with
134+
descriptive names into 40 characters:
135+
136+
````
137+
✅ CharCount.ContArr.Str.kr
138+
✅ Seq.prefixWhile.AnySeq.UnfoldSeq.lazy
139+
````
140+
141+
As a last resort, use *numbered suffixes* to disambiguate between benchmarks
142+
with minor implementation variations.
143+
144+
</details></li>
145+
</ul>
146+
147+
Technically, the benchmark's name must match the following regular expression:
148+
`[A-Z][a-zA-Z0-9\-\.!?]+`

‎benchmark/scripts/Benchmark_Driver

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ class BenchmarkDoctor(object):
293293
self.log.addHandler(self.console_handler)
294294
self.log.debug('Checking tests: %s', ', '.join(self.driver.tests))
295295
self.requirements = [
296-
self._name_matches_capital_words_convention,
296+
self._name_matches_benchmark_naming_convention,
297297
self._name_is_at_most_40_chars_long,
298298
self._no_setup_overhead,
299299
self._reasonable_setup_time,
@@ -305,20 +305,29 @@ class BenchmarkDoctor(object):
305305
"""Unregister handler on exit."""
306306
self.log.removeHandler(self.console_handler)
307307

308-
capital_words_re = re.compile('[A-Z][a-zA-Z0-9]+')
308+
benchmark_naming_convention_re = re.compile(r'[A-Z][a-zA-Z0-9\-\.!?]+')
309+
camel_humps_re = re.compile(r'[a-z][A-Z]')
309310

310311
@staticmethod
311-
def _name_matches_capital_words_convention(measurements):
312+
def _name_matches_benchmark_naming_convention(measurements):
312313
name = measurements['name']
313-
match = BenchmarkDoctor.capital_words_re.match(name)
314+
match = BenchmarkDoctor.benchmark_naming_convention_re.match(name)
314315
matched = match.group(0) if match else ''
316+
composite_words = len(BenchmarkDoctor.camel_humps_re.findall(name)) + 1
315317

316318
if name != matched:
317319
BenchmarkDoctor.log_naming.error(
318-
"'%s' name doesn't conform to UpperCamelCase convention.",
320+
"'%s' name doesn't conform to benchmark naming convention.",
319321
name)
320322
BenchmarkDoctor.log_naming.info(
321-
'See http://bit.ly/UpperCamelCase')
323+
'See http://bit.ly/BenchmarkNaming')
324+
325+
if composite_words > 4:
326+
BenchmarkDoctor.log_naming.warning(
327+
"'%s' name is composed of %d words.", name, composite_words)
328+
BenchmarkDoctor.log_naming.info(
329+
"Split '%s' name into dot-separated groups and variants. "
330+
"See http://bit.ly/BenchmarkNaming", name)
322331

323332
@staticmethod
324333
def _name_is_at_most_40_chars_long(measurements):

‎benchmark/scripts/test_Benchmark_Driver.py

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -509,10 +509,14 @@ def test_measure_10_independent_1s_benchmark_series(self):
509509
'Measuring B1, 5 x i1 (2048 samples), 5 x i2 (2048 samples)'],
510510
self.logs['debug'])
511511

512-
def test_benchmark_name_matches_capital_words_conventions(self):
512+
def test_benchmark_name_matches_naming_conventions(self):
513513
driver = BenchmarkDriverMock(tests=[
514514
'BenchmarkName', 'CapitalWordsConvention', 'ABBRName',
515-
'wrongCase', 'Wrong_convention'])
515+
'TooManyCamelCaseHumps',
516+
'Existential.Array.method.1x.Val4',
517+
'Flatten.Array.Array.Str.for-in.reserved',
518+
'Flatten.Array.String?.as!.NSArray',
519+
'wrongCase', 'Wrong_convention', 'Illegal._$%[]<>{}@^()'])
516520
with captured_output() as (out, _):
517521
doctor = BenchmarkDoctor(self.args, driver)
518522
doctor.check()
@@ -522,12 +526,22 @@ def test_benchmark_name_matches_capital_words_conventions(self):
522526
self.assertNotIn('BenchmarkName', output)
523527
self.assertNotIn('CapitalWordsConvention', output)
524528
self.assertNotIn('ABBRName', output)
529+
self.assertNotIn('Existential.Array.method.1x.Val4', output)
530+
self.assertNotIn('Flatten.Array.Array.Str.for-in.reserved', output)
531+
self.assertNotIn('Flatten.Array.String?.as!.NSArray', output)
532+
err_msg = " name doesn't conform to benchmark naming convention."
525533
self.assert_contains(
526-
["'wrongCase' name doesn't conform to UpperCamelCase convention.",
527-
"'Wrong_convention' name doesn't conform to UpperCamelCase "
528-
"convention."], self.logs['error'])
534+
["'wrongCase'" + err_msg, "'Wrong_convention'" + err_msg,
535+
"'Illegal._$%[]<>{}@^()'" + err_msg], self.logs['error'])
529536
self.assert_contains(
530-
['See http://bit.ly/UpperCamelCase'], self.logs['info'])
537+
["'TooManyCamelCaseHumps' name is composed of 5 words."],
538+
self.logs['warning'])
539+
self.assert_contains(
540+
['See http://bit.ly/BenchmarkNaming'], self.logs['info'])
541+
self.assert_contains(
542+
["Split 'TooManyCamelCaseHumps' name into dot-separated groups "
543+
"and variants. See http://bit.ly/BenchmarkNaming"],
544+
self.logs['info'])
531545

532546
def test_benchmark_name_is_at_most_40_chars_long(self):
533547
driver = BenchmarkDriverMock(tests=[

0 commit comments

Comments
 (0)