dotnet / jitutils

MIT License
144 stars 59 forks source link

Put names output by jit-tp-analyze on the right #392

Closed jakobbotsch closed 6 months ago

jakobbotsch commented 7 months ago

Since the name field is always the longest I think it makes things easier to read if it is on the right.

Example base:

Base: 10885388824, Diff: 10964148407, +0.7235%

public: void __cdecl GcInfoEncoder::Build(void)                                                                                                                                                                                                                                                                 : 23837106 : +39.24%    : 30.26% : +0.2190%
public: void __cdecl GCInfo::gcInfoRecordGCRegStateChange(class GcInfoEncoder *, enum GCInfo::MakeRegPtrMode, unsigned int, unsigned __int64, enum GcSlotState, unsigned __int64, unsigned __int64 *)                                                                                                           : 14893444 : +40.24%    : 18.91% : +0.1368%
private: class JitHashTable<struct RegSlotIdKey, struct RegSlotIdKey, unsigned int, class CompAllocator, class JitHashTableBehavior>::Node * __cdecl JitHashTable<struct RegSlotIdKey, struct RegSlotIdKey, unsigned int, class CompAllocator, class JitHashTableBehavior>::FindNode(struct RegSlotIdKey) const : 6632146  : +57.73%    : 8.42%  : +0.0609%
public: void __cdecl GCInfo::gcMakeRegPtrTable(class GcInfoEncoder *, unsigned int, unsigned int, enum GCInfo::MakeRegPtrMode, unsigned int *)                                                                                                                                                                  : 5489031  : +9.74%     : 6.97%  : +0.0504%
public: void __cdecl BitStreamWriter::Write(unsigned __int64, unsigned int)                                                                                                                                                                                                                                     : 5034101  : +11.69%    : 6.39%  : +0.0462%
jitstd::`anonymous namespace'::quick_sort<GcInfoEncoder::LifetimeTransition *,CompareLifetimeTransitionsByOffsetThenSlot>                                                                                                                                                                                       : 4867161  : +37.58%    : 6.18%  : +0.0447%
public: void __cdecl GcInfoEncoder::SetSlotState(unsigned int, unsigned int, enum GcSlotState)                                                                                                                                                                                                                  : 3845160  : +65.06%    : 4.88%  : +0.0353%
public: void * __cdecl ArenaAllocator::allocateMemory(unsigned __int64)                                                                                                                                                                                                                                         : 2557227  : +0.89%     : 3.25%  : +0.0235%
memset                                                                                                                                                                                                                                                                                                          : 1918044  : +3.68%     : 2.43%  : +0.0176%
public: int __cdecl BitStreamWriter::EncodeVarLengthUnsigned(unsigned __int64, unsigned int)                                                                                                                                                                                                                    : 1518588  : +9.78%     : 1.93%  : +0.0140%
memcmp                                                                                                                                                                                                                                                                                                          : 1356191  : +1769.19%  : 1.72%  : +0.0125%
public: bool __cdecl SimplerHashTable<class BitArray const *, class LiveStateFuncs, unsigned int, class GcInfoHashBehavior>::Set(class BitArray const *, unsigned int)                                                                                                                                          : 1160836  : +41018.94% : 1.47%  : +0.0107%
private: void __cdecl JitHashTable<struct RegSlotIdKey, struct RegSlotIdKey, unsigned int, class CompAllocator, class JitHashTableBehavior>::Grow(void)                                                                                                                                                         : 995520   : +286.15%   : 1.26%  : +0.0091%
public: void __cdecl SimplerHashTable<class BitArray const *, class LiveStateFuncs, unsigned int, class GcInfoHashBehavior>::Reallocate(unsigned int)                                                                                                                                                           : 886635   : +39884.62% : 1.13%  : +0.0081%
jitstd::`anonymous namespace'::quick_sort<GcSlotDescAndId *,CompareSlotDescAndIdBySlotDesc>                                                                                                                                                                                                                     : 870613   : +7.23%     : 1.11%  : +0.0080%
private: void __cdecl GcInfoEncoder::SizeofSlotStateVarLengthVector(class BitArray const &, unsigned int, unsigned int, unsigned int *, unsigned int *, unsigned int *)                                                                                                                                         : 856702   : +15.58%    : 1.09%  : +0.0079%
public: void __cdecl emitter::emitStackPopLargeStk(unsigned char *, bool, unsigned char, unsigned int)                                                                                                                                                                                                          : 546505   : +10.41%    : 0.69%  : +0.0050%
public: void __cdecl BitStreamWriter::CopyTo(unsigned char *)                                                                                                                                                                                                                                                   : 441137   : +11.51%    : 0.56%  : +0.0041%
memcpy_$fo_default$                                                                                                                                                                                                                                                                                             : 423954   : +3.73%     : 0.54%  : +0.0039%
public: virtual void * __cdecl CompIAllocator::Alloc(unsigned __int64)                                                                                                                                                                                                                                          : 257960   : +23.30%    : 0.33%  : +0.0024%
public: virtual void * __cdecl CompIAllocator::ArrayAlloc(unsigned __int64, unsigned __int64)                                                                                                                                                                                                                   : 248880   : +39884.62% : 0.32%  : +0.0023%
__security_check_cookie                                                                                                                                                                                                                                                                                         : 94854    : +0.27%     : 0.12%  : +0.0009%

Example diff:

Base: 10885388824, Diff: 10964148407, +0.7235%

23837106 : +39.24%    : 30.26% : +0.2190% : public: void __cdecl GcInfoEncoder::Build(void)                                                                                                                                                                                                                                                                
14893444 : +40.24%    : 18.91% : +0.1368% : public: void __cdecl GCInfo::gcInfoRecordGCRegStateChange(class GcInfoEncoder *, enum GCInfo::MakeRegPtrMode, unsigned int, unsigned __int64, enum GcSlotState, unsigned __int64, unsigned __int64 *)                                                                                                          
6632146  : +57.73%    : 8.42%  : +0.0609% : private: class JitHashTable<struct RegSlotIdKey, struct RegSlotIdKey, unsigned int, class CompAllocator, class JitHashTableBehavior>::Node * __cdecl JitHashTable<struct RegSlotIdKey, struct RegSlotIdKey, unsigned int, class CompAllocator, class JitHashTableBehavior>::FindNode(struct RegSlotIdKey) const
5489031  : +9.74%     : 6.97%  : +0.0504% : public: void __cdecl GCInfo::gcMakeRegPtrTable(class GcInfoEncoder *, unsigned int, unsigned int, enum GCInfo::MakeRegPtrMode, unsigned int *)                                                                                                                                                                 
5034101  : +11.69%    : 6.39%  : +0.0462% : public: void __cdecl BitStreamWriter::Write(unsigned __int64, unsigned int)                                                                                                                                                                                                                                    
4867161  : +37.58%    : 6.18%  : +0.0447% : jitstd::`anonymous namespace'::quick_sort<GcInfoEncoder::LifetimeTransition *,CompareLifetimeTransitionsByOffsetThenSlot>                                                                                                                                                                                      
3845160  : +65.06%    : 4.88%  : +0.0353% : public: void __cdecl GcInfoEncoder::SetSlotState(unsigned int, unsigned int, enum GcSlotState)                                                                                                                                                                                                                 
2557227  : +0.89%     : 3.25%  : +0.0235% : public: void * __cdecl ArenaAllocator::allocateMemory(unsigned __int64)                                                                                                                                                                                                                                        
1918044  : +3.68%     : 2.43%  : +0.0176% : memset                                                                                                                                                                                                                                                                                                         
1518588  : +9.78%     : 1.93%  : +0.0140% : public: int __cdecl BitStreamWriter::EncodeVarLengthUnsigned(unsigned __int64, unsigned int)                                                                                                                                                                                                                   
1356191  : +1769.19%  : 1.72%  : +0.0125% : memcmp                                                                                                                                                                                                                                                                                                         
1160836  : +41018.94% : 1.47%  : +0.0107% : public: bool __cdecl SimplerHashTable<class BitArray const *, class LiveStateFuncs, unsigned int, class GcInfoHashBehavior>::Set(class BitArray const *, unsigned int)                                                                                                                                         
995520   : +286.15%   : 1.26%  : +0.0091% : private: void __cdecl JitHashTable<struct RegSlotIdKey, struct RegSlotIdKey, unsigned int, class CompAllocator, class JitHashTableBehavior>::Grow(void)                                                                                                                                                        
886635   : +39884.62% : 1.13%  : +0.0081% : public: void __cdecl SimplerHashTable<class BitArray const *, class LiveStateFuncs, unsigned int, class GcInfoHashBehavior>::Reallocate(unsigned int)                                                                                                                                                          
870613   : +7.23%     : 1.11%  : +0.0080% : jitstd::`anonymous namespace'::quick_sort<GcSlotDescAndId *,CompareSlotDescAndIdBySlotDesc>                                                                                                                                                                                                                    
856702   : +15.58%    : 1.09%  : +0.0079% : private: void __cdecl GcInfoEncoder::SizeofSlotStateVarLengthVector(class BitArray const &, unsigned int, unsigned int, unsigned int *, unsigned int *, unsigned int *)                                                                                                                                        
546505   : +10.41%    : 0.69%  : +0.0050% : public: void __cdecl emitter::emitStackPopLargeStk(unsigned char *, bool, unsigned char, unsigned int)                                                                                                                                                                                                         
441137   : +11.51%    : 0.56%  : +0.0041% : public: void __cdecl BitStreamWriter::CopyTo(unsigned char *)                                                                                                                                                                                                                                                  
423954   : +3.73%     : 0.54%  : +0.0039% : memcpy_$fo_default$                                                                                                                                                                                                                                                                                            
257960   : +23.30%    : 0.33%  : +0.0024% : public: virtual void * __cdecl CompIAllocator::Alloc(unsigned __int64)                                                                                                                                                                                                                                         
248880   : +39884.62% : 0.32%  : +0.0023% : public: virtual void * __cdecl CompIAllocator::ArrayAlloc(unsigned __int64, unsigned __int64)                                                                                                                                                                                                                  
94854    : +0.27%     : 0.12%  : +0.0009% : __security_check_cookie                                                                                                                                                                                                                                                                                        

cc @dotnet/jit-contrib @SingleAccretion any opposing opinions? If so I can add an option, but if everyone agrees...

SingleAccretion commented 7 months ago

any opposing opinions?

I would like to register one: I like the method names first because they're usually the most interesting thing to look at.

I would not have any objections to making the change's output the default, though.

tannergooding commented 7 months ago

Is there a balance we could strike here?

Maybe

ShortName | InsCountDiff | InsPercentageDiff | ContributionPercentage | FullName

So for example, rather than:

public: void __cdecl GcInfoEncoder::Build(void)                                                                                                                                                                                                                                                                 : 23837106 : +39.24%    : 30.26% : +0.2190%

or

23837106 : +39.24%    : 30.26% : +0.2190% : public: void __cdecl GcInfoEncoder::Build(void) 

We could do:

GcInfoEncoder::Build                          : 23837106 : +39.24%    : 30.26% : +0.2190% : public: void __cdecl GcInfoEncoder::Build(void)

Which avoids parameters/return/types/etc shifting important data extremely far to the right, while still giving quick contextual access to a key piece of information (the name)

SingleAccretion commented 7 months ago

I don't think there is a need to complicate the tool with method name parsing, which is not always possible. The method name could be any of "fully mangled", "lightly demangled" (like in the documentation for the tool), "fully demangled" (like in Jakob's example), or even "missing" (if you don't have the debug symbols for whichever reason).

jakobbotsch commented 6 months ago

I'll just keep this locally.