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

Add documentation for Hermes sampling profiler #2159

Merged
merged 21 commits into from
Sep 24, 2020

Conversation

jessieAnhNguyen
Copy link
Contributor

@jessieAnhNguyen jessieAnhNguyen commented Aug 20, 2020

Summary:


This PR adds a new doc for Hermes sampling profiler. This is built as part of the goal to visualize the performance of JavaScript running on Hermes in a React Native app. @saphal1998 and I worked on this under @axemclion and @jevakallio guidance.

Test Plan:


  • We followed the instruction on Making a change
  • The new doc looks like this
    image

@facebook-github-bot
Copy link

Hi @jessieAnhNguyen!

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@react-native-bot
Copy link

react-native-bot commented Aug 20, 2020

Deploy preview for react-native ready!

Built with commit c1c0fc2

https://deploy-preview-2159--react-native.netlify.app

Changes to docs/ are reflected in the next "master" version.

Thank you for your contributions.

How to ContributeDocumentation Sources

@Simek
Copy link
Collaborator

Simek commented Aug 20, 2020

Hi @jessieAnhNguyen, thank you so much for this article, it is a great addition! 👍

Unfortunately there are some minor display issues on the newly added page (due to Markdown parsing by Docususaurus):
Annotation 2020-08-20 103615

Let me know if you will have any problems fixing it, I can try to help, preview:
https://deploy-preview-2159--react-native.netlify.app/docs/next/profile-hermes

@saphal1998
Copy link
Contributor

@Simek, thank you so much for getting back to us so soon. We have resolved the markdown parsing issue, the PR should be ready for review! 🎉

@Simek
Copy link
Collaborator

Simek commented Aug 20, 2020

@saphal1998 After the tweaks, from the technical stand point this PR LGTM, but since it is a bigger addition I have asked @rachelnabors to review it too 🙂

Copy link
Contributor

@rachelnabors rachelnabors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a PR of a few changes—great stuff! I would like to go through one more time, but first, please check that the step-by-step instructions conform to our menu/command styleguides. Thank you!

jessieAnhNguyen and others added 3 commits August 21, 2020 16:40
@jessieAnhNguyen
Copy link
Contributor Author

jessieAnhNguyen commented Aug 21, 2020

Hi @rachelnabors, thank you a lot for giving the feedback! @saphal1998 and I have merged your PR and worked to edit the wording more based on the menu/command guidelines. Please review our changes and let us know if we should change anything else 🎉

@Simek
Copy link
Collaborator

Simek commented Aug 21, 2020

@jessieAnhNguyen According to the latest changes in #2157: Can you move the added images to website/static/docs/assets/?

@jessieAnhNguyen
Copy link
Contributor Author

Hi @Simek I've moved the images to the new directory 😄 Thanks for pointing that out!

@Simek Simek added the Guest Docs New docs written by invited niche expert community members label Sep 7, 2020
@rachelnabors
Copy link
Contributor

Hey there, I PRed some changes to this PR. Could you merge them so we can merge this? Thanks!

@saphal1998
Copy link
Contributor

Thanks for the review! We have merged the changes @rachelnabors

Copy link
Contributor

@rachelnabors rachelnabors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. One last thing: Remove the first image under "enable sampling profiler"—it doesn't really explain anything.

@davelyon
Copy link

Thanks so much for documenting this, it's been immensely helpful for me! I wanted to share some feedback around the "Open the downloaded profile in Chrome DevTools" step. I haven't been able to get the regular devtools to correctly open the file, but I have been consistently able to get them open and viewable by using the tracing view at chrome://tracing and loading there.

I may have misunderstood how to get to the right state with the Chrome DevTools, so it might be that some clarification is needed on that step, otherwise I would suggest that the documentation direct users to the chrome://tracing page.

Thanks again for documenting this! 👍

@jessieAnhNguyen
Copy link
Contributor Author

Hi @rachelnabors, I've deleted the image. Thanks for reviewing!

@saphal1998
Copy link
Contributor

Hi @davelyon, so chrome://tracing opens the profile even without transforming it (i.e. the raw .cpuprofile file), however in that case, we cannot interpret much from the visualisation. This is why we opted for the visualisation in the Performance tab. So, can you, if possible, post the image of your performance profile on chrome://tracing and Chrome DevTools so that we can compare the two? Or even validate if they look identical?

I just want to check if the transformation took place. If the transformation did take place, then the visualisation on both of these should be identical according to us.

@rachelnabors
Copy link
Contributor

Hey hey! Since @davelyon hasn't responded, I'm going to merge your PR. (If future edits are necessary, you can always edit the doc or file an issue!)

Thank you for your patience and hard work on this! Your contribution has made the docs more useful to more people :)

2020_me_thanks

@rachelnabors rachelnabors merged commit 8e37222 into facebook:master Sep 24, 2020
slorber pushed a commit to react-native-website-migration/react-native-website that referenced this pull request Oct 9, 2020
* Update introduction.md (facebook#2181)

Hooks were introduced in React Native 0.59, not 0.58.

* adding snack player instead of images for flexbox (facebook#2171)

* adding snack player instead of images for flexbox

* chore: revert yarn.lock

* minor changes and improvements, prettier run on examples

Co-authored-by: Bartosz Kaszubowski <gosimek@gmail.com>

* UI & UX Design with InputAccessoryView (facebook#2183)

* UI Design with InputAccessoryView 

* Update inputaccessoryview.md

* small tweaks

Co-authored-by: Bartosz Kaszubowski <gosimek@gmail.com>

* update TextInput example, refs facebook#2187

* add TOC and improve formatting on the Android publishing page

* Modernize code examples in the direct manipulation section (facebook#2189)

* Modernize code examples in the direct manipulation section.

* small code tweaks/fixes + run code through Prettier

Co-authored-by: Bartosz Kaszubowski <gosimek@gmail.com>

* update Alert API page, include iOS specific functions from deprecated AlertIOS (facebook#2184)

Thanks for this one, @Simek!

* Update getting-started.md (facebook#2194)

Python2 & Python3 are both compatible

* fix homepage showcase app list graphical issue (facebook#2185)

Co-authored-by: R Nabors <rachelnabors@users.noreply.github.com>

* docs: add matrix to transform (facebook#2197)

* Bump decompress from 4.2.0 to 4.2.1 in /website (facebook#2200)

Bumps [decompress](https://github.com/kevva/decompress) from 4.2.0 to 4.2.1.
- [Release notes](https://github.com/kevva/decompress/releases)
- [Commits](kevva/decompress@v4.2.0...v4.2.1)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Typo fix (facebook#2202)

* fixes facebook#2201, delete duplicated TOC, add note about removal to TabBarIOS pages

* docs: update deprecated usage of findNodeHandle in measureLayout (facebook#2199)

* docs: update deprecated usage of findNodeHandle in measureLayout

* minor text and Snack tweaks, prettier run

Co-authored-by: Bartosz Kaszubowski <gosimek@gmail.com>

* bump alex and node-fetch, fix new alex warning

* typo fix

* TextInput: Add `onPressIn` and `onPressOut` docs (facebook#2205)

Original summary by @yungsters in [1b994f9](facebook/react-native@b7b0e23):
Introduces support for `onPressIn` and `onPressOut` on the `TextInput` component.

This makes it possible to add visual feedback when users touch interact with `TextInput` components.

* Fix typos (facebook#2204)

* link out-of-tree platforms, add macOS platform, unify dashes on homepage (facebook#2186)

* link out-of-tree platforms, add macOS, unify dashes on homepage

* revert dashes changes

* update SectionList page, extract ViewToken, add labels (facebook#2191)

* update TextInput press callbacks type, refs facebook#2205

* Added workarounds for TextInputs (facebook#2203)

* Added workarounds for TextInputs

* provide info on the focus management using <Modal>

* small text tweaks

Co-authored-by: Bartosz Kaszubowski <gosimek@gmail.com>

* update Text page and add types, add LayoutEvent object page (facebook#2209)

* Incorrect command (facebook#2207)

https://github.com/react-native-community/cli/blob/master/docs/commands.md#run-android

`--variant [string]`

* Updated ScrollView `disableIntervalMomentum` doc. (facebook#2211)

* Remove announcement banner. (facebook#2213)

* Docs: Removing `onTextInput` prop from TextInput (facebook#2216)

* Removing `onTextInput` prop from TextInput doc.

* Removing `onTextInput` prop from TextInput on version 0.62 & 0.63.

* Signing not under General for me (facebook#2214)

* Update running-on-simulator-ios.md (facebook#2220)

We have to specify the generation for the iPhone SE simulator.

* [DOC] fixed errata in image doc (facebook#2221)

* Add documentation for Hermes sampling profiler (facebook#2159)

Co-authored-by: Saphal Patro <31125345+saphal1998@users.noreply.github.com>
Co-authored-by: R Nabors <rachelnabors@users.noreply.github.com>
Co-authored-by: Saphal Patro <saphal1998@gmail.com>

* Adding Android NDK Installation Guide (facebook#2225)

* Docs: Integration with android fragment (facebook#2217)

Co-authored-by: Sebastien Bailouni <sbailouni@gmail.com>

* Add new option to ActionSheetIOS called disabledButtonsIndices (facebook#1898)

* Adding configureNext.onAnimationDidFail callback on LayoutAnimation (facebook#2136)

Co-authored-by: Christoph Nakazawa <cpojer@fb.com>

* Added Coinbase to showcase (facebook#2227)

* Added Coinbase to showcase

* Add Coinbase icon

* 0.63.3 release - update Pressable delay (facebook#2226)

* Improved README.md (facebook#2233)

* fix typo (facebook#2232)

* fix typo

fixes typo in "Using Custom Path Aliases with TypeScript" section (configure -> configuring)

* sentence rewrite

Co-authored-by: Bartosz Kaszubowski <gosimek@gmail.com>

* Add Shopify and Tableau to showcase (facebook#2228)

* Add Shopify in showcase

* Add Shopify icon

* Add Tableau to showcase

* Add tableau image asset

* Update Tableau icon

* updated debugging.md (facebook#2241)

fixed a typo in unhandled error

* Update appregistry.md (facebook#2246)

* Update out-of-tree-platforms.md (facebook#2243)

* Remove unnecessary python dependency (facebook#2250)

Python is not required anymore on windows platform. The only binary addon for react-native project are some deps of `ws`, [which now have prebuilt binary and are optional](https://github.com/websockets/ws#opt-in-for-performance-and-spec-compliance). 
Tested on a brand new windows 10 image with only node installed, the initiating process goes well.

* Update network.md to show android non ssl block (facebook#2245)

* Update network.md to show android non ssl block

added note for android's android:usesCleartextTraffic flag in manifest file to allow non ssl encrypted networking - to go alongside the existing note for ios.

* small text tweaks

Co-authored-by: Bartosz Kaszubowski <gosimek@gmail.com>

* remove link in native-modules-ios (facebook#2244)

Co-authored-by: luism3861 <eduardo_media@fragua.com.mx>

* Update textinput.md (facebook#2249)

* Update textinput.md

This will make it more clear for beginners who are trying to use onBlur to do a final action on their textInput's text value.

* formatting, text tweaks

Co-authored-by: Bartosz Kaszubowski <gosimek@gmail.com>

* revert change to Netlify config due to V2 test merge

Co-authored-by: Georgios Kotziabassis <kotziabassis@gmail.com>
Co-authored-by: Muhammad Saeed <37156636+Stringsaeed@users.noreply.github.com>
Co-authored-by: Bartosz Kaszubowski <gosimek@gmail.com>
Co-authored-by: Mojtaba Vandaei <62926492+mojvan@users.noreply.github.com>
Co-authored-by: Steven Luscher <steveluscher@users.noreply.github.com>
Co-authored-by: Pratik Khandelwal <prkhandelwal@hotmail.com>
Co-authored-by: R Nabors <rachelnabors@users.noreply.github.com>
Co-authored-by: Jesse Katsumata <jesse.katsumata@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Joe Bernard <joe@joebernard.com>
Co-authored-by: Thibault Malbranche <thibault.malbranche@epitech.eu>
Co-authored-by: Luis Miguel Alvarado <luismiguel1730@gmail.com>
Co-authored-by: Pramod Kotipalli <p13i@users.noreply.github.com>
Co-authored-by: Madd.is <github@madd.is>
Co-authored-by: Kirill <laikisounds@gmail.com>
Co-authored-by: Ashwin Mothilal <ashwin.mothilal2124@gmail.com>
Co-authored-by: Christoph Nakazawa <cpojer@fb.com>
Co-authored-by: captDaylight <paul.christophe6@gmail.com>
Co-authored-by: ppichier <pierantonio.pichierri@gmail.com>
Co-authored-by: Varunendra Pratap Singh <vpsingh016@gmail.com>
Co-authored-by: Jessie Anh Nguyen <47696418+jessieAnhNguyen@users.noreply.github.com>
Co-authored-by: Saphal Patro <31125345+saphal1998@users.noreply.github.com>
Co-authored-by: Saphal Patro <saphal1998@gmail.com>
Co-authored-by: Agastya Darma <gedeagas22@gmail.com>
Co-authored-by: Steven Martin <stevenisekimartin@gmail.com>
Co-authored-by: Sebastien Bailouni <sbailouni@gmail.com>
Co-authored-by: Luke Walczak <lukasz.walczak.pwr@gmail.com>
Co-authored-by: Jessica Lin <jlin2700@gmail.com>
Co-authored-by: Vishal Vishwakarma <56310842+vishalvishw10@users.noreply.github.com>
Co-authored-by: Adam Hunter <arhunter@gmail.com>
Co-authored-by: Jessica Lin <jplin@fb.com>
Co-authored-by: Ashish Shakya <41261918+Ashish1101@users.noreply.github.com>
Co-authored-by: Sunny Luo <sunnylqm@gmail.com>
Co-authored-by: Alexander Sklar <asklar@microsoft.com>
Co-authored-by: s-maurice <51819025+s-maurice@users.noreply.github.com>
Co-authored-by: luism3861 <36824170+luism3861@users.noreply.github.com>
Co-authored-by: luism3861 <eduardo_media@fragua.com.mx>
Co-authored-by: Ralph Maamari <ralphpal@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Guest Docs New docs written by invited niche expert community members
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants