Feat heap visualization #59
Conversation
|
Wow, thanks! Let me know when you're ready for me to take a look at this / give it a spin 🙂 |
|
I currently implement the very simple heap viz, and I'm optimizing the details, refactoring to extract common code and further testing. This should take a few days |
|
Sounds good, let me know if you want any pointers/guidance |
|
|
||
| public add(rootPath: string | undefined, location: ILocation) { | ||
| public add(rootPath: string | undefined, node: NodeType) { | ||
| const expand = once(() => { |
There was a problem hiding this comment.
Because src is generated according to callFrame, I changed the logic of this method to judge src first, if there is src, add it and then return. Previously, two annotations would be added where both have src and callFrame.
And I changed new Position(Math.max(0, src.lineNumber - 1), Math.max(0, src.columnNumber - 1)) to new Position(Math.max(0, src.lineNumber) , Math.max(0, src.columnNumber)) because according to my tests, the position without -1 is correct.
Am I overlooking some special case here?
There was a problem hiding this comment.
I find that there need to fix lineNumber and columnNumber in model, instead of fix annotations. And I fixed it in 3036c87
|
I've done coding and self-testing, you can try out the features and review the code if you have time. I'm not familiar with notebooks, so I didn't do heap visualizations for notebooks. I put the shared parts of the previous code into common, and the modified files are formatted by the Prettier extension when they are saved. Some newly added files I'm not sure whether to use camelCase or kebab-case, please let me know if you need to unify. Please let me know what else needs to be modified or improved. thanks! |
| !src || | ||
| src.source.sourceReference !== 0 || | ||
| !src.source.path || | ||
| src.source.path === node.callFrame.url |
There was a problem hiding this comment.
Because we've set annotation of node.callFrame.url before. If the src.source.path and node.callFrame.url are the same, and we don't check, the annotation will be added twice.
| import { ITreeNode } from 'vscode-js-profile-core/out/esm/heap/model'; | ||
| import styles from '../common/time-view.css'; | ||
|
|
||
| const useTimeViewRow = <T extends IGraphNode | ITreeNode>({ |
There was a problem hiding this comment.
These should be written as components, not hooks. I will refactor them.
There was a problem hiding this comment.
I've made these changes to the CPU profiler in b702150. If you'd like to tackle something like that on the flame chart that would be helpful, otherwise I'll get to that in a day or two 🙂
There was a problem hiding this comment.
Great, I'll try to refactor the flame chart this week.
|
Thanks! I'll work on getting this merged |
| '@typescript-eslint/no-use-before-define': 'off', | ||
| '@typescript-eslint/explicit-function-return-type': 'off', | ||
| '@typescript-eslint/interface-name-prefix': ['error', { prefixWithI: 'always' }], | ||
| 'react/display-name': 'off', |
There was a problem hiding this comment.
If this rule is enabled, anonymous components (e.g.: (props) => component inside const makeFoo = () => (props) => component) will report an error.
|
Published! Thanks for your work here! |
2 participants
No description provided.