Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable hierarchy-based link resolver implementation by default #498

Merged
merged 5 commits into from
Mar 10, 2023

Conversation

d-ronnqvist
Copy link
Contributor

@d-ronnqvist d-ronnqvist commented Mar 3, 2023

Bug/issue #, if applicable: rdar://98692937

Summary

This enabled the hierarchy-based link resolver by default. It also updates a handful of symbol links to remove disambiguation that's now redundant.

Dependencies

None

Testing

  1. Unset the feature flag if you have it locally set: defaults remove -g DocCUseHierarchyBasedLinkResolver
  2. Build the TestBundle.docc: swift run docc convert ./Tests/SwiftDocCTests/Test\ Bundles/TestBundle.docc

You should see error messages from the new link resolver—meaning that the feature is enabled by default—for example:

Reference at '/Test-Bundle/article2' can't resolve 'badlink'. No similar pages.

Checklist

Make sure you check off the following items. If they cannot be completed, provide a reason.

  • [ ] Added tests
  • Ran the ./bin/test script and it succeeded
  • [ ] Updated documentation if necessary

@d-ronnqvist
Copy link
Contributor Author

In my local benchmarking the new resolver is 2-3% faster (not just link resolution but the full build) at the cost of ~1.5% memory usage.

The 'duration-finalize-navigation-index' samples from the 'before' measurements show possible signs of bias.
The benchmark diff analysis assumes that these values follow a normal distribution around the value that they're meant to represent. If they don't the conclusion from the diff analysis could be invalid.
A human should inspect these values. If the samples look biased or look too noisy you can try gathering new metrics with more 'repetitions' or with a larger .docc catalog.
Full precision values: [0.12006933333759662, 0.11944316666631494, 0.12049266666872427, 0.11977662499703001, 0.11928495833126362, 0.12023837500601076, 0.12203124999359716, 0.12072566666756757, 0.12110733332519885, 0.12103204165759962, 0.12071654167084489, 0.12063270833459683, 0.12081491666322108, 0.12121604166168254, 0.12179474999720696]
████████████████████████████████████████████████████████████████████████████████   0.12 sec
███████████████████████████████████████████████████████████████████████████████▋   0.119 sec
████████████████████████████████████████████████████████████████████████████████▍   0.12 sec
███████████████████████████████████████████████████████████████████████████████▉   0.12 sec
███████████████████████████████████████████████████████████████████████████████▌   0.119 sec
████████████████████████████████████████████████████████████████████████████████   0.12 sec
█████████████████████████████████████████████████████████████████████████████████▍   0.122 sec
████████████████████████████████████████████████████████████████████████████████▌   0.121 sec
████████████████████████████████████████████████████████████████████████████████▊   0.121 sec
████████████████████████████████████████████████████████████████████████████████▊   0.121 sec
████████████████████████████████████████████████████████████████████████████████▌   0.121 sec
████████████████████████████████████████████████████████████████████████████████▌   0.121 sec
████████████████████████████████████████████████████████████████████████████████▌   0.121 sec
████████████████████████████████████████████████████████████████████████████████▉   0.121 sec
█████████████████████████████████████████████████████████████████████████████████▎   0.122 sec

The 'peak-memory' samples from the 'after' measurements show possible signs of bias.
The benchmark diff analysis assumes that these values follow a normal distribution around the value that they're meant to represent. If they don't the conclusion from the diff analysis could be invalid.
A human should inspect these values. If the samples look biased or look too noisy you can try gathering new metrics with more 'repetitions' or with a larger .docc catalog.
Full precision values: [1068862592, 1060441408, 1057590336, 1061784704, 1060605184, 1056541952, 1063390656, 1071058240, 1057557568, 1055051136, 1045630144, 1057951040, 1053428800, 1046957312, 1057590464]
████████████████████████████████████████████████████████████████████████████████   1,019.3 MB
███████████████████████████████████████████████████████████████████████████████▍   1,011.3 MB
███████████████████████████████████████████████████████████████████████████████▎   1,008.6 MB
███████████████████████████████████████████████████████████████████████████████▌   1,012.6 MB
███████████████████████████████████████████████████████████████████████████████▌   1,011.5 MB
███████████████████████████████████████████████████████████████████████████████   1,007.6 MB
███████████████████████████████████████████████████████████████████████████████▋   1,014.1 MB
████████████████████████████████████████████████████████████████████████████████▎   1,021.4 MB
███████████████████████████████████████████████████████████████████████████████▎   1,008.6 MB
███████████████████████████████████████████████████████████████████████████████   1,006.2 MB
██████████████████████████████████████████████████████████████████████████████▍   997.2 MB
███████████████████████████████████████████████████████████████████████████████▎   1,008.9 MB
██████████████████████████████████████████████████████████████████████████████▉   1,004.6 MB
██████████████████████████████████████████████████████████████████████████████▍   998.5 MB
███████████████████████████████████████████████████████████████████████████████▎   1,008.6 MB

┌──────────────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Metric                                   │ Change          │ old-resolver         │ new-resolver         │
├──────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ Duration for 'bundle-registration'       │ -4.785%¹        │ 5.972 sec            │ 5.687 sec            │
│ Duration for 'convert-total-time'        │ -2.772%²        │ 9.815 sec            │ 9.543 sec            │
│ Duration for 'documentation-processing'  │ no change³      │ 3.603 sec            │ 3.617 sec            │
│ Duration for 'finalize-navigation-index' │ no change⁴,⁵    │ 0.121 sec            │ 0.123 sec            │
│ Peak memory footprint                    │ +1.43%⁶,⁷       │ 995 MB               │ 1,009.3 MB           │
│ Data subdirectory size                   │ no change       │ 280.4 MB             │ 280.4 MB             │
│ Index subdirectory size                  │ no change       │ 2.5 MB               │ 2.5 MB               │
│ Total DocC archive size                  │ no change       │ 371.7 MB             │ 371.7 MB             │
│ Topic Anchor Checksum                    │ no change       │ a8c40d797627683c5d7c │ a8c40d797627683c5d7c │
│ Topic Graph Checksum                     │ no change       │ da82bd7f35eae4bb56e8 │ da82bd7f35eae4bb56e8 │
└──────────────────────────────────────────────────────────────────────────────────────────────────────────┘

 1: There's a statistically significant difference between the two benchmark measurements.
    The values are different enough that the most probable explanation is that they're random samples from two different data sets.
    t-statistic : +15.106         degrees of freedom : 28       95% confidence critical value : +2.042

 2: There's a statistically significant difference between the two benchmark measurements.
    The values are different enough that the most probable explanation is that they're random samples from two different data sets.
    t-statistic : +11.117         degrees of freedom : 28       95% confidence critical value : +2.042

 3: No statistically significant difference.
    The values are similar enough that the most probable explanation is that they're random samples from the same data set.
    t-statistic : -0.990          degrees of freedom : 28       95% confidence critical value : +2.042

 4: No statistically significant difference.
    The values are similar enough that the most probable explanation is that they're random samples from the same data set.
    t-statistic : -1.492          degrees of freedom : 28       95% confidence critical value : +2.042

 5: A human should check that the measurements for this metric look like random samples around a certain value to ensure the validity of this result.


 6: There's a statistically significant difference between the two benchmark measurements.
    The values are different enough that the most probable explanation is that they're random samples from two different data sets.
    t-statistic : -4.792          degrees of freedom : 28       95% confidence critical value : +2.042

 7: A human should check that the measurements for this metric look like random samples around a certain value to ensure the validity of this result.

@d-ronnqvist d-ronnqvist marked this pull request as ready for review March 8, 2023 00:19
@d-ronnqvist
Copy link
Contributor Author

@swift-ci please test

@d-ronnqvist d-ronnqvist merged commit 24ba4b7 into swiftlang:main Mar 10, 2023
@d-ronnqvist d-ronnqvist changed the title [Hold] Enable hierarchy-based link resolver implementation by default Enable hierarchy-based link resolver implementation by default Mar 10, 2023
@d-ronnqvist d-ronnqvist deleted the enable-link-resolver-feature branch March 10, 2023 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants