ENH: Allow checking vmprof output in progress. #139
Conversation
| #ifndef RPYTHON_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?
| fileobj = self.fileobj | ||
|
|
||
| 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
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? ;)