ENH: Allow checking vmprof output in progress.#139
Uh oh!
There was an error while loading. Please reload this page.
Conversation
| #ifndefRPYTHON_VMPROF | ||
| PyObject*all_done_uids=NULL; | ||
| if (all_code_uids!=NULL){ | ||
| all_done_uids=PySet_New(NULL); |
There was a problem hiding this comment.
in this function we cant use CPython API. It will not work on PyPy. I have used khash for the native symbols already (to avoid duplication). We can reuse khash for that.
There was a problem hiding this comment.
Yeah, here's my confusion - we already use CPython API further down the line, similarly if an #ifndef RPYTHON_VMPROF block. :(
...so, is the right fix to change both?
Or is the other one special enough to be kept around?
| self.detect_file_sizes() | ||
| self.read_static_header() | ||
| function_lookup={ |
There was a problem hiding this comment.
Should have been done this way some time ago...
| self.profile_memory=False | ||
| self.profile_lines=None | ||
| self.profile_memory=None | ||
| self.adr_size=None |
There was a problem hiding this comment.
can we change adr_size or remove it? it is simply the len of state.virtual_ips... it should not be confused with the byte size of the addresses contained in the profile. We could turn that into a method with a longer name...
There was a problem hiding this comment.
Ah, that particular thing I added literally only for one test, I suppose we could rework that test instead to use a lower-level API?
mattip changed the title 2 participants
This PR isn't quite ready yet, but I thought I'd start early, then we can talk about the preferred approach as I'm hacking away. Related to #127.
Two improvements are made so far:
Reasoning:
The current implementation of (2) seems slightly iffy to me, is it okay to use a PySet here, or should this be ported over to the khash thing? Any other comments? ;)