Overhauled allocations documentation for helpfulness #643
Conversation
RobinBateman808
requested review from
0xTim,
alexandersandberg,
daveverwer,
dempseyatgithub,
parispittman,
kaishin,
shahmishal,
cthielen and
federicobucchi
as code owners
|
@0xTim - made changes as suggested and corrected code formatting. |
|
|
||
| ```swift | ||
| An example program using AsyncHTTPClient can be written as: | ||
| ``` |
There was a problem hiding this comment.
Missing language on the code block, re-add
There was a problem hiding this comment.
@0xTim - Thanks for the review. I'm not sure what's happening with the code. I'm not making any changes to the blocks or text unless it was something you previously caught. Any suggestions or insight would be appreciated. :-)
There was a problem hiding this comment.
Adjusted space before code blocks. Preview of text renders language in code block.
There was a problem hiding this comment.
I checked the code block against the original. Looks the same.
|
|
||
| ```bash | ||
| In this instance, a user probe was installed for all allocation functions because Swift uses other functions like `calloc` and `posix_memalign`. | ||
| ``` |
There was a problem hiding this comment.
Missing language in the code block
There was a problem hiding this comment.
Adjusted space before code blocks. The text renders as it should.
|
|
||
| which should output something like | ||
| To confirm the user probe `probe_libc:malloc` works, run this command: | ||
| ``` |
There was a problem hiding this comment.
| ``` | |
| ```bash |
| ```bash | ||
| ### Creating flame graphs | ||
| Once you’ve successfully recorded data using `perf record`, you can invoke the following command to produce an SVG file with the flame graph: | ||
| ``` |
There was a problem hiding this comment.
| ``` | |
| ```bash |
There was a problem hiding this comment.
Apologies, I missed this. Has been corrected and checked 2x.
| On Apple platforms, Swift uses a slightly larger number of allocation functions than Linux. Therefore, specifying a few more functions is required. | ||
|
|
||
| Once the data is collected, run this command to create an SVG file: | ||
| ``` |
There was a problem hiding this comment.
Not sure what you mean here. But removed "Therefore, specifying a few more functions is required." to mitigate confusion.
| perf script | \ | ||
| /FlameGraph/stackcollapse-perf.pl - | \ | ||
| swift demangle --simplified | \ | ||
| /FlameGraph/flamegraph.pl --countname allocations \ | ||
| --width 1600 > out.svg | ||
| --width 1600 > out.svg |
There was a problem hiding this comment.
Still missing the indentation
There was a problem hiding this comment.
Added indent again. Hoping it "sticks" this time.
Co-authored-by: Tim Condon <[email protected]>
Co-authored-by: Tim Condon <[email protected]>
Co-authored-by: Tim Condon <[email protected]>
|
@0xTim - Hopefully, the latest changes will fix things. Thanks for your time and patience! :-) |
|
Hi @0xTim - Friendly bump when you have a moment. :-) |
There was a problem hiding this comment.
Sorryfor the delay @RobinBateman808
For language I mean the language of the code in the code block. So if you start a clock of code with
```swift
That tells the site generator to add the language="swift" tag to the generated code block so the syntax highlighter can process the code block and show syntax highlighting. This appears to have been removed in many of the code blocks so we should add them back to make it easier to read. Some options are
```swift
```docker
```bash
| ```bash | ||
| ### Creating flame graphs | ||
| Once you’ve successfully recorded data using `perf record`, you can invoke the following command to produce an SVG file with the flame graph: | ||
| ``` |
```swift ```bash
Co-authored-by: Tim Condon <[email protected]>
There was a problem hiding this comment.
Thanks! This looks good to go
There are a few existing code blocks for running commands that could be annotated with
```bash
But since they're existing it shouldn't hold up the PR (note that the output of the different commands shouldn't have a language applied) and can be done in a separate PR if desired
|
@swift-ci test |
|
@swift-ci test |
|
Thanks @0xTim! I'll create a new PR to remove the applied language for the outputs. |
|
@RobinBateman808 It's all good, there wasn't any applied languages and I added all the missing ones before merging as it was a quick fix |
|
Ack. Thanks! |
Motivation:
Edited documentation to improve helpfulness for new and experienced Swift on Server (SOS) users.
Modifications:
Added definitions and explanations where needed, edited existing content for clarity, and formatted for scannability.
Result:
Increase user adoption and bolster the SOS community.