Page MenuHomePhabricator

[xray] Add xray-ignore-loops option
ClosedPublic

Authored by ianlevesque on Jan 13 2020, 3:27 PM.

Details

Summary

XRay allows tuning by minimum function size, but also always instruments
functions with loops in them. If the minimum function size is set to a
large value the loop instrumention ends up causing most functions to be
instrumented anyway. This adds a new flag, xray-ignore-loops, to disable
the loop detection logic.

Diff Detail

Event Timeline

ianlevesque created this revision.Jan 13 2020, 3:27 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 13 2020, 3:27 PM

Remove extraneous changes

smeenai resigned from this revision.EditedJan 13 2020, 5:30 PM

To make sure I'm understanding this correctly, previously, for any function with a loop in it, XRay would ignore the minimum instruction count threshold and instrument it unconditionally. You're adding an option to make XRay respect the threshold even if the function has loops. Is that correct?

I'm wondering what the reason is for the existing logic ignoring loops, and if we can incorporate loops into the existing logic instead of adding another option (and whether that would be better). I'm sure @dberris can speak more to that, so leaving this to him.

dberris accepted this revision.Jan 14 2020, 12:17 AM

LGTM

I'm wondering how you're imbuing these attributes to the functions from the front-ends -- is this something you intend to support as a flag in Clang for example?

To make sure I'm understanding this correctly, previously, for any function with a loop in it, XRay would ignore the minimum instruction count threshold and instrument it unconditionally. You're adding an option to make XRay respect the threshold even if the function has loops. Is that correct?

I'm wondering what the reason is for the existing logic ignoring loops, and if we can incorporate loops into the existing logic instead of adding another option (and whether that would be better). I'm sure @dberris can speak more to that, so leaving this to him.

Because we assume that loops that haven't been optimised out might have variable runtime, we ensure that any code that has loops might have interesting performance characteristics and always instrument those. That might have been a conservative assumption, and it might be a bit hard to manually mark functions that must never be instrumented instead, so I understand the need for something like this attribute to be more targeted.

This revision is now accepted and ready to land.Jan 14 2020, 12:17 AM

I'm wondering how you're imbuing these attributes to the functions from the front-ends -- is this something you intend to support as a flag in Clang for example?

Yes exactly. I will have the Clang diff up shortly.

This revision was automatically updated to reflect the committed changes.