[Lldb-commits] [PATCH] D32823: Remove an expensive lock from Timer

[Jim Ingham via lldb-commits]

I'd vote for keeping the timers if possible.  Their job is to tell you "of the time spent in some operation, how much was spent in say DWARF parsing", etc.  That has been useful on occasion.

Jim

> On May 4, 2017, at 2:12 PM, Scott Smith via Phabricator via lldb-commits <lldb-commits at lists.llvm.org> wrote:
> 
> scott.smith marked 3 inline comments as done.
> scott.smith added a comment.
> 
> In https://reviews.llvm.org/D32823#745799, @labath wrote:
> 
>> Seems reasonable, however: I am not sure who actually uses these timers. I'd be tempted to just remove the timers that are causing the contention.
> 
> 
> IMO we should either keep the timers and make them fast, or get rid of all timers.  I don't like the idea of training developers to know that timers are slow, and should be used sparingly.  Especially since the fix is relatively simple.
> 
> Though an atomic skip list would be another way to improve performance without requiring changing each callsite; the downside is the lg-n lookup when tallying the time.
> 
> 
> 
> ================
> Comment at: source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:5421
>         char *buf = (char *) malloc (ident_command.cmdsize);
> -        if (buf != nullptr 
> +        if (buf != nullptr
>             && m_data.CopyData (offset, ident_command.cmdsize, buf) == ident_command.cmdsize) {
> ----------------
> labath wrote:
>> I am not sure how you format your changes, but you should make sure you format only the lines you've touched, and not the whole files. git-clang-format <https://github.com/llvm-mirror/clang/blob/master/tools/clang-format/git-clang-format> will do that for you -- when you set it up, you just run `git clang-format HEAD^` and it will format your last patch).
> Yeah that'll be helpful.  I have default .emacs settings for work that don't necessarily apply well to the llvm codebase, this tool will make it easier to fix up.
> 
> 
> Repository:
>  rL LLVM
> 
> https://reviews.llvm.org/D32823
> 
> 
> 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits