Hi,
The following condition in function_instance::merge
if (other->timestamp() < timestamp())
  set_timestamp (other->timestamp())

would set timestamp() to 0 if other->timestamp() was 0 effectively
dropping the symbol from time-profile based reordering resulting in 
substantially reduced performance.

The patch fixes this by ensuring other->timestamp() is greater than zero before 
assigning it to timestamp.
Also, handles the case when timestamp() is zero but other->timestamp() is 
non-zero.

For a large internal workload, this resulted in avoiding around ~67 symbols 
from getting dropped which were present in gcov file and included in function 
reordering,
which resulted in ~10% improvement.
Would the patch be OK to commit to trunk, given that it's pretty localized and 
gated on -fauto-profile ?

PS: There is also another issue I noticed about same filename having different 
indices in autofdo_source_profile::map_ which makes find_function_instance 
return NULL
even if the symbol/filename entry is present in gcov file but that's an 
orthogonal issue.

Thanks,
Prathamesh

Reply via email to