Add visualization event handlers to Connections class #933
Conversation
|
By analyzing the blame information on this pull request, we identified @chetan51, @akhilaananthram and @rcrowder to be potential reviewers |
|
I verified that it [seems to be] performance-neutral. |
|
@mrcslws merge in master |
|
@mrcslws ping |
| return token; | ||
| } | ||
|
|
||
| void Connections::unsubscribe(UInt32 token) |
There was a problem hiding this comment.
Would it be easier if subscribe took a char* name argument that you also pass to unsubscribe? That way the client just has to use the same string for both calls rather than keep track of the token returned.
There was a problem hiding this comment.
Fair point. But then:
- This class would have to store copies of strings.
- The caller would have to be careful not to use strings that are used elsewhere. When I grab HTM traces, I do it via a set of independent classes that are unaware of each other. It'd be easy for one of these classes to accidentally stomp on another class's event handler. To avoid this, each tracing class would have to generate a string and store it.
This "token" approach mimics javascript's setTimeout and setInterval API.
|
Done with review. Looks like the SWIG directors are the right thing now that I looked into it more. |
|
WARNING! This Pull Request has been inactive for 25 days, and will be automatically closed in 5 days if not updated before then. This is an automated message. |
Fixes #932