Skip to content

Commit 5154886

Browse files
atrickatrick
authored
Merge pull request #20334 from palimondo/within-cells-interlinked
[benchmark] Naming Convention
2 parents 5e15018 + 419ec8b commit 5154886

File tree

3 files changed

+186
-12
lines changed

3 files changed

+186
-12
lines changed

‎benchmark/Naming.md

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
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. Stick to ASCII letters, numbers and period.
37+
Exceptionally:
38+
39+
* Use **`-`** only to denote control flow constructs like `for-in` or `if-let`.
40+
* Use **`!`** and **`?`** for optional types, conditional or forced downcasting,
41+
optional chaining etc.
42+
43+
````
44+
✅ Array.append.Array.Int?
45+
✅ Flatten.Array.Tuple4.for-in.reserved
46+
✅ Bridging.NSArray.as!.Array.NSString
47+
````
48+
49+
Note: Special characters that could be interpreted by the shell require escaping
50+
(`\!`) or quoting the name, when running such benchmarks individually.
51+
52+
</details><p><!-- spacer --></p></li>
53+
<li>
54+
<strong>Use groups and variants</strong> to structure the benchmark family by
55+
degrees of freedom (e.g. different types implementing a protocol, value vs.
56+
reference types etc.). Use <strong>numbered suffixes</strong> to denote
57+
differently sized variants.
58+
<details>
59+
60+
Benchmarks in a family can be grouped by the tested operation, method or varied
61+
by types and different workload sizes. It might be necessary to abbreviate some
62+
names to fit the size limit, based on the longest combination. Choose consistent
63+
names for the components throughout all members in the family, to allow for
64+
relative comparison across the different axis of variation.
65+
66+
````
67+
✅ Seq.dropFirst.Array
68+
✅ Seq.dropLast.Range.lazy
69+
✅ Seq.dropWhile.UnfoldSeq
70+
✅ Seq.prefix.AnySeq.RangeIter.lazy
71+
✅ Seq.prefixWhile.AnyCol.Array
72+
✅ Seq.suffix.AnySeq.UnfoldSeq.lazy
73+
74+
✅ Existential.Array.ConditionalShift.Ref1
75+
✅ Existential.Array.Mutating.Ref2
76+
✅ Existential.Array.method.1x.Ref3
77+
✅ Existential.Array.method.2x.Ref4
78+
✅ Existential.Array.Shift.Val0
79+
✅ Existential.MutatingAndNonMutating.Val1
80+
✅ Existential.Mutating.Val2
81+
✅ Existential.method.1x.Val3
82+
✅ Existential.method.2x.Val4
83+
✅ Existential.Pass2.method.1x.Ref1
84+
✅ Existential.Pass2.method.2x.Ref2
85+
86+
✅ Set.isSubset.Int25
87+
✅ Set.symmetricDifference.Int50
88+
````
89+
90+
</details><p><!-- spacer --></p></li>
91+
<li>
92+
<strong>Groups and types are <code>UpperCase</code>, methods are
93+
<code>lowerCase</code>.</strong>
94+
<details>
95+
96+
Use periods to separate the name components in variants derived from specialised
97+
generic types or significant method chains.
98+
99+
````
100+
⛔️ InsertCharacterTowardsEndIndexNonASCII
101+
````
102+
103+
There's no need to be literal with type names. **Be descriptive**:
104+
105+
````
106+
✅ String.insert.EmojiChar.NearEnd
107+
✅ String.insert.ASCIIChar.StartIndex
108+
✅ Flatten.Array.Tuple4.lazy.flatMap
109+
````
110+
111+
</details><p><!-- spacer --></p></li>
112+
<li>
113+
<strong>Keep it short.</strong> 40 characters at most. Abbreviate if necessary.
114+
<details>
115+
116+
Benchmarking results are reported on GitHub and very long names are causing
117+
horizontal table scrolling which unfortunately obscures the columns with actual
118+
measurements. Fixed upper size limit also helps with the formatted console
119+
output, when measuring locally. *It is more important for benchmark's name to be
120+
unique and short, than overly descriptive.*
121+
122+
Prefer concise names for potential benchmark family extensions. Leave out the
123+
nested types from variants if they aren't strictly necessary for disambiguation.
124+
If there's potentially valuable future variant, like testing `ContiguousArray`,
125+
keep the `Array` now, allowing for addition of `ContArr` variants later.
126+
127+
Use **`Val`** and **`Ref`** as short descriptors for variants that compare value
128+
types (`struct`, `Int`) with reference types (often named with `Class` in the
129+
legacy-style).
130+
Prefer **`Char`** to `Character`, which can be combined with codepage or
131+
language prefix/suffix when necessary (`EmojiChar`, `ASCIIChar`). For benchmarks
132+
that measure `String`'s Unicode performance for various languages, use
133+
the [three letter codes](https://en.wikipedia.org/wiki/ISO_639-3) instead of
134+
spelling out the whole language names.
135+
136+
When necessary, use *consistent* abbreviations like `Str` and `Arr` within the
137+
benchmark family, to fit a system with descriptive names into 40 characters:
138+
139+
````
140+
✅ Bridging.NSDict.as!.Dict.NSString.NSNum
141+
✅ Seq.prefixWhile.AnySeq.UnfoldSeq.lazy
142+
````
143+
144+
As a last resort, use *numbered suffixes* to disambiguate between benchmarks
145+
with minor implementation variations.
146+
147+
</details></li>
148+
</ul>
149+
150+
Technically, the benchmark's name must match the following regular expression:
151+
`[A-Z][a-zA-Z0-9\-.!?]+`

‎benchmark/scripts/Benchmark_Driver

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ class BenchmarkDoctor(object):
338338
self.log.addHandler(self.console_handler)
339339
self.log.debug('Checking tests: %s', ', '.join(self.driver.tests))
340340
self.requirements = [
341-
self._name_matches_capital_words_convention,
341+
self._name_matches_benchmark_naming_convention,
342342
self._name_is_at_most_40_chars_long,
343343
self._no_setup_overhead,
344344
self._reasonable_setup_time,
@@ -351,20 +351,29 @@ class BenchmarkDoctor(object):
351351
for handler in list(self.log.handlers):
352352
handler.close()
353353

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

356357
@staticmethod
357-
def _name_matches_capital_words_convention(measurements):
358+
def _name_matches_benchmark_naming_convention(measurements):
358359
name = measurements['name']
359-
match = BenchmarkDoctor.capital_words_re.match(name)
360+
match = BenchmarkDoctor.benchmark_naming_convention_re.match(name)
360361
matched = match.group(0) if match else ''
362+
composite_words = len(BenchmarkDoctor.camel_humps_re.findall(name)) + 1
361363

362364
if name != matched:
363365
BenchmarkDoctor.log_naming.error(
364-
"'%s' name doesn't conform to UpperCamelCase convention.",
366+
"'%s' name doesn't conform to benchmark naming convention.",
365367
name)
366368
BenchmarkDoctor.log_naming.info(
367-
'See http://bit.ly/UpperCamelCase')
369+
'See http://bit.ly/BenchmarkNaming')
370+
371+
if composite_words > 4:
372+
BenchmarkDoctor.log_naming.warning(
373+
"'%s' name is composed of %d words.", name, composite_words)
374+
BenchmarkDoctor.log_naming.info(
375+
"Split '%s' name into dot-separated groups and variants. "
376+
"See http://bit.ly/BenchmarkNaming", name)
368377

369378
@staticmethod
370379
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
@@ -564,10 +564,14 @@ def test_measure_10_independent_1s_benchmark_series(self):
564564
'Measuring B1, 5 x i1 (200 samples), 5 x i2 (200 samples)'],
565565
self.logs['debug'])
566566

567-
def test_benchmark_name_matches_capital_words_conventions(self):
567+
def test_benchmark_name_matches_naming_conventions(self):
568568
driver = BenchmarkDriverMock(tests=[
569569
'BenchmarkName', 'CapitalWordsConvention', 'ABBRName',
570-
'wrongCase', 'Wrong_convention'])
570+
'TooManyCamelCaseHumps',
571+
'Existential.Array.method.1x.Val4',
572+
'Flatten.Array.Array.Str.for-in.reserved',
573+
'Flatten.Array.String?.as!.NSArray',
574+
'wrongCase', 'Wrong_convention', 'Illegal._$%[]<>{}@^()'])
571575
with captured_output() as (out, _):
572576
doctor = BenchmarkDoctor(self.args, driver)
573577
doctor.check()
@@ -577,12 +581,22 @@ def test_benchmark_name_matches_capital_words_conventions(self):
577581
self.assertNotIn('BenchmarkName', output)
578582
self.assertNotIn('CapitalWordsConvention', output)
579583
self.assertNotIn('ABBRName', output)
584+
self.assertNotIn('Existential.Array.method.1x.Val4', output)
585+
self.assertNotIn('Flatten.Array.Array.Str.for-in.reserved', output)
586+
self.assertNotIn('Flatten.Array.String?.as!.NSArray', output)
587+
err_msg = " name doesn't conform to benchmark naming convention."
580588
self.assert_contains(
581-
["'wrongCase' name doesn't conform to UpperCamelCase convention.",
582-
"'Wrong_convention' name doesn't conform to UpperCamelCase "
583-
"convention."], self.logs['error'])
589+
["'wrongCase'" + err_msg, "'Wrong_convention'" + err_msg,
590+
"'Illegal._$%[]<>{}@^()'" + err_msg], self.logs['error'])
584591
self.assert_contains(
585-
['See http://bit.ly/UpperCamelCase'], self.logs['info'])
592+
["'TooManyCamelCaseHumps' name is composed of 5 words."],
593+
self.logs['warning'])
594+
self.assert_contains(
595+
['See http://bit.ly/BenchmarkNaming'], self.logs['info'])
596+
self.assert_contains(
597+
["Split 'TooManyCamelCaseHumps' name into dot-separated groups "
598+
"and variants. See http://bit.ly/BenchmarkNaming"],
599+
self.logs['info'])
586600

587601
def test_benchmark_name_is_at_most_40_chars_long(self):
588602
driver = BenchmarkDriverMock(tests=[

0 commit comments

Comments
 (0)