User Details
- User Since
- Jan 12 2017, 6:57 PM (291 w, 4 d)
Aug 9 2018
Thought that occurred to me this morning:
Aug 8 2018
Suggestion for splitting this up.
Aug 6 2018
Consider creating wrapper functions around DataExtractor's getU64, getU32, getU16, and getU8 that take a DataExtractor&, an Offset&, a descriptive twine, and return an Expected<T>. I'm trying to come up with ideas to keep all the granular checks as you desire, but pull some of the ceremonial noise out of the flow of the main logic.
Aug 5 2018
Aug 3 2018
Aug 2 2018
This is way easier to read now. Did you sanity check loading a trace into chrome trace viewer that it outputs?
Please consider my comments about the tests before pushing.
Jul 31 2018
Haven't read the test yet. Logic looks sound.
Jul 19 2018
Publishing existing comments. Haven't reviewed the whole patch.
Jul 12 2018
Jul 11 2018
Just one concern about exit handler being too aggressive if normal operation finalized and flushed.
Jul 10 2018
Is this still a work in progress patch as noted? I'll make some time to look at this today.
Jul 9 2018
Seems like half of this change is DCHECKS. Much better coverage! Was that just a result of your strategy for diagnosing what actually caused the problems and leaving those in?
Jun 11 2018
Jun 8 2018
I'm going to have to download the renamed files to diff locally. Is there a way to do this in Differential that I'm missing?
Jun 6 2018
Jun 4 2018
May 30 2018
Aside from the discussion about a more invasive change to the way cooperative state transitions, this is fine. Thanks for writing the test for serialization.
May 23 2018
LGTM, once the dependent changes get in.
in the chain* I meant.
This one is more straightforward than the previous in the change. The main ideas all fall into place nicely, but I have pointed out some details that could use some attention.
May 17 2018
My overall feedback about this code is that after reading it, it probably deserves another pass over just to focus on what allocates and frees and which state transitions are responsible for closing the loops.
May 14 2018
May 13 2018
This was easier to review than I expected. I found it easier to follow than the allocator/array CL. Sorry for the long delay!
May 3 2018
LGTM.
Can you add some Reports, which could be guarded behind sanitizer verbose, for warning messages when users do deprecated things? This looks sensible, but a bit of nagging incentives people to change.
Everything looks good except I'd like to see us flexible for mode configuration that may contain nulls.
Apr 28 2018
This looks better now, although there were a few issues that slipped through the tests and I wonder if there are others lurking.
Apr 22 2018
Finally reached the end. Phew!
Apr 19 2018
Haven't made any changes yet. Thanks for the comments Dean.
Apr 18 2018
Haven't looked at segmented array yet, but I had a good look at the allocator. I think I caught one serious bug, and the rest is just suggestions.
Apr 17 2018
I'm going to land this and the llvm/clang changes and in a separate CL make some improvements as discussed offline for event type registration (handling deduping event descriptors).
Undoing formatting change.
Apr 16 2018
My editor got a bit carried away with automatically clang-formatting lib/CodeGen/CodeGenFunction.cpp. I'll fix that so that I'm not messing up the revision history.
Added flags and bundle options.
Adding a comment to the test to encourage getting the event types from
compiler-rt
Apr 15 2018
Apr 13 2018
Adding some comments for questions I had. Please chime in if you can shed light on these.
Apr 12 2018
[XRay] [compiler-rt] Implement trampoline and handler for typed xray event tracing.
These comments were dangling for a while, so I'm posting an update. Currently compiling llvm with a new intrinsic to match. Fingers crossed!
[XRay] [compiler-rt] Implement trampoline and handler for typed xray event tracing.
Thanks for the cleanup.
Mar 6 2018
Good point about preserving the size and structure of XRayLogImpl for ABI compatibility. Piper monorepo has lulled me into a false sense of security!
Feb 26 2018
Thanks for the feedback Dean.
Mostly looks good, but I think you should consider making the buffer iterator interface part of XRayLogImpl or some other mechanism to associate it with a logging mode.
Feb 23 2018
- Jump 20
Feb 22 2018
Dec 13 2017
Thanks for pointing out the scope cleanup from MProtectHelper and making some improvements/clarifications.
Suggestion for future changes:
Nov 6 2017
Nov 2 2017
Here are the revisions pre-landing. I'll probably land this tomorrow and follow up with a separate review of simple lit test cases for these conversions.
- Phabricator comments.
Nov 1 2017
Made some changes, particularly regarding string formatting.
Oct 29 2017
Thanks for taking a look Dean. Here's a link to an example trace that can be loaded up at chrome://tracing. I'll respond to your other comments tomorrow.
Oct 26 2017
Oct 19 2017
Oct 12 2017
Updating with Dean's review comments.
Oct 6 2017
Sep 7 2017
Sep 5 2017
Switch from std::for_each to range based for.
David Blakie convinced me over email that std::for_each doesn't add value over ranged based for.
Sep 2 2017
Update the way an iterator type is referenced to not make assumptions about references.