Page MenuHomePhabricator

[lldb/Core] Change large function threshold variable into a setting.
ClosedPublic

Authored by mib on Feb 25 2021, 9:52 AM.

Details

Summary

This patch replaces the static large function threshold variable with a
global debugger setting (stop-disassembly-max-size).

The default threshold is now set to 32KB (instead of 8KB) and can be modified.

rdar://74726362

Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>

Diff Detail

Event Timeline

mib requested review of this revision.Feb 25 2021, 9:52 AM
mib created this revision.
JDevlieghere added inline comments.Feb 25 2021, 10:24 AM
lldb/include/lldb/Core/Debugger.h
249

This seems like it could have a better name, how about DisassemblyTreshold? If this only used when stopped (like StopDisassemblyCount) then maybe add the Stop prefix here too.

mib updated this revision to Diff 326450.Feb 25 2021, 11:20 AM
mib marked an inline comment as done.

Renamed setting to stop-disassembly-size.

mib edited the summary of this revision. (Show Details)Feb 25 2021, 11:23 AM
JDevlieghere added inline comments.Feb 25 2021, 11:37 AM
lldb/source/Core/CoreProperties.td
31

Nit, let's move this next to StopDisassemblyCount and StopDisassemblyDisplay.

31

Sorry to go on about this but now it doesn't convey that it's the maximum size. Maybe stop-disassembly-max-size or stop-disassembly-size-threshold.

mib updated this revision to Diff 326478.Feb 25 2021, 1:30 PM
mib edited the summary of this revision. (Show Details)

Address @JDevlieghere comments.

JDevlieghere accepted this revision.Feb 25 2021, 1:31 PM

LGTM. Thanks for bearing with me!

This revision is now accepted and ready to land.Feb 25 2021, 1:31 PM
This revision was landed with ongoing or failed builds.Feb 25 2021, 1:35 PM
This revision was automatically updated to reflect the committed changes.
labath added a subscriber: labath.Mar 2 2021, 12:11 AM

As the author of this logic, I want to note that the original limit I chose was very arbitrary, and 32k (or any other small-ish number would work equally well). The main objective was to disable accidental multi-megabyte disassemblies (which can happen when the object file has no symbol information).

I'm not actually opposed to the, just wanting to make sure you're not adding it on my account.

I'm not actually opposed to the, just wanting to make sure you're not adding it on my account.

to the setting

mib added a comment.Mar 2 2021, 1:33 AM

As the author of this logic, I want to note that the original limit I chose was very arbitrary, and 32k (or any other small-ish number would work equally well). The main objective was to disable accidental multi-megabyte disassemblies (which can happen when the object file has no symbol information).

I'm not actually opposed to the, just wanting to make sure you're not adding it on my account.

I made that change after being requested to increase the limit. I thought turning it into a setting gives the user more flexibility.