On Mon, Nov 16, 2020 at 10:14:19AM +0800, Zhang Yuhang wrote:

On Sat, Nov 14, 2020 at 10:42:59AM +0800, Lu Kaiyi wrote:

...

[why] Currently, the compare output result default order by alphabet. If there are too many lines in result, it will be difficult for user to find the lines that starts with $suite in metric column. So, need reorder compare output result with keeping $suite item on the top of the table.

[how] Divide the lines into two parts before print according to whether the line start with $suite or not, then combine output result into a whole to highlight the important info.

[example] command: compare "id=z9.148096" "id=z9.148143" in z9 suites_list:["sysbench-mysql"]

[before]

              0                               1  metric

---

     %stddev       change            %stddev
        \             |                 \
 224.42            -74.1%         58.14          boot-time.boot

20400.95 -73.5% 5397.13 boot-time.idle 11.56 +83.3% 21.19 boot-time.kernel_boot 29424.00 -100.0% 0.00 interrupts.17:GICv3.23.Level.arm-pmu 4100080.00 -99.8% 10196.00 interrupts.226:ITS-MSI.65536001.Edge.hns3-0000:7d:00.0-TxRx-0 ...

1266042.00 -18.3% 1034206.00 softirqs.RCU 547849.00 -27.4% 397593.00 softirqs.SCHED 159990.00 -80.5% 31181.00 softirqs.TASKLET 1617967.00 -23.0% 1245331.00 softirqs.TIMER 2149.23 -100.0% 0.00 sysbench-mysql.time.elapsed_time 2149.23 -100.0% 0.00 sysbench-mysql.time.elapsed_time.max 214458.00 -100.0% 0.00 sysbench-mysql.time.involuntary_context_switches 2.00 -100.0% 0.00 sysbench-mysql.time.major_page_faults 192832.00 -100.0% 0.00 sysbench-mysql.time.maximum_resident_set_size 90746.00 -100.0% 0.00 sysbench-mysql.time.minor_page_faults 65536.00 -100.0% 0.00 sysbench-mysql.time.page_size 201.00 -100.0% 0.00 sysbench-mysql.time.percent_of_cpu_this_job_got 2796.60 -100.0% 0.00 sysbench-mysql.time.system_time 1535.98 -100.0% 0.00 sysbench-mysql.time.user_time 282616989.00 -100.0% 0.00 sysbench-mysql.time.voluntary_context_switches 2149.23 -100.0% 0.00 time.elapsed_time 2149.23 -100.0% 0.00 time.elapsed_time.max 214458.00 -100.0% 0.00 time.involuntary_context_switches 2.00 -100.0% 0.00 time.major_page_faults 192832.00 -100.0% 0.00 time.maximum_resident_set_size ...

[after]

              0                               1  metric

---

     %stddev       change            %stddev
        \             |                 \
2149.23           -100.0%          0.00          sysbench-mysql.time.elapsed_time
2149.23           -100.0%          0.00          sysbench-mysql.time.elapsed_time.max

214458.00 -100.0% 0.00 sysbench-mysql.time.involuntary_context_switches 2.00 -100.0% 0.00 sysbench-mysql.time.major_page_faults 192832.00 -100.0% 0.00 sysbench-mysql.time.maximum_resident_set_size 90746.00 -100.0% 0.00 sysbench-mysql.time.minor_page_faults 65536.00 -100.0% 0.00 sysbench-mysql.time.page_size 201.00 -100.0% 0.00 sysbench-mysql.time.percent_of_cpu_this_job_got 2796.60 -100.0% 0.00 sysbench-mysql.time.system_time 1535.98 -100.0% 0.00 sysbench-mysql.time.user_time 282616989.00 -100.0% 0.00 sysbench-mysql.time.voluntary_context_switches 224.42 -74.1% 58.14 boot-time.boot 20400.95 -73.5% 5397.13 boot-time.idle 11.56 +83.3% 21.19 boot-time.kernel_boot 29424.00 -100.0% 0.00 interrupts.17:GICv3.23.Level.arm-pmu 4100080.00 -99.8% 10196.00 interrupts.226:ITS-MSI.65536001.Edge.hns3-0000:7d:00.0-TxRx-0 2894304.00 +12.8% 3265730.00 interrupts.4:GICv3.26.Level.arch_timer 11546998.00 -100.0% 0.00 interrupts.746:ITS-MSI.99092481.Edge.hns3-0000:bd:00.1-TxRx-0 7476064.00 -100.0% 0.00 interrupts.747:ITS-MSI.99092482.Edge.hns3-0000:bd:00.1-TxRx-2 ...

Signed-off-by: Lu Kaiyi 2392863668@qq.com

lib/compare.rb | 16 ++++++++++------ lib/compare_matrixes.rb | 41 ++++++++++++++++++++++++++++------------- lib/matrix2.rb | 15 +++++++++++---- 3 files changed, 49 insertions(+), 23 deletions(-)

diff --git a/lib/compare.rb b/lib/compare.rb index 645f682..813e08c 100644 --- a/lib/compare.rb +++ b/lib/compare.rb @@ -20,8 +20,8 @@ require 'yaml'

def compare_matrices_list(argv, common_conditions, options) condition_list = parse_argv(argv, common_conditions)

• matrices_list = create_matrices_list(condition_list)
• compare_matrixes(matrices_list, options: options)

• matrices_list, suites_list = create_matrices_list(condition_list)
• compare_matrixes(matrices_list, suites_list, options: options)

end

def parse_argv(argv, common_conditions) @@ -37,12 +37,16 @@ end

def create_matrices_list(conditions) matrices_list = []

• suites_list = [] es = ESQuery.new(ES_HOST, ES_PORT) conditions.each do |condition| query_results = es.multi_field_query(condition)

• matrices_list << combine_query_data(query_results)

• matrix, suites = combine_query_data(query_results)
• matrices_list << matrix
• suites_list.concat(suites) end

• matrices_list

• return matrices_list, suites_list

end

# ------------------------------------------------------------------------------------------- @@ -58,8 +62,8 @@ end def compare_group(argv, dimensions, options) conditions = parse_conditions(argv) dims = dimensions.split(' ')

• groups_matrices = create_groups_matrices_list(conditions, dims)
• compare_group_matrices(groups_matrices, options)

• groups_matrices, suites_list = create_groups_matrices_list(conditions, dims)
• compare_group_matrices(groups_matrices, suites_list, options)

end

def create_groups_matrices_list(conditions, dims) diff --git a/lib/compare_matrixes.rb b/lib/compare_matrixes.rb index 119d42d..bd1e646 100644 --- a/lib/compare_matrixes.rb +++ b/lib/compare_matrixes.rb @@ -128,7 +128,7 @@ def set_compare_values(index, values, field, success, options) end

def get_values_by_field(matrixes_list, field, matrixes_size, success, options)

• # get values by field, values struce example: values[0][:average]

• # get values by field, values struct example: values[0][:average] # values = {} matrixes_list.length.times do |index|

@@ -193,7 +193,7 @@ def matrixes_empty?(matrixes_list) return matrixes_list.any?(&:empty?) end

-def compare_matrixes(matrixes_list, matrixes_titles = nil, group_key = nil, options: {}) +def compare_matrixes(matrixes_list, suites_list, matrixes_titles = nil, group_key = nil, options: {}) # compare matrix in matrixes_list and print info # @matrixes_list: list consisting of matrix # @matrixes_titles: number or dimension of matrix @@ -208,6 +208,7 @@ def compare_matrixes(matrixes_list, matrixes_titles = nil, group_key = nil, opti result_str = group_key ? "\n\n\n\n\n" + group_key : '' result_str += get_all_result_str( matrixes_values,

• suites_list, matrixes_titles, matrixes_list.size, options[:theme]

@@ -382,7 +383,7 @@ end # compare each matrices_list within pre dimension of group matrices # input: group matrices # output: pre compare result of each group -def compare_group_matrices(group_matrices, options) +def compare_group_matrices(group_matrices, suites_list, options) result_str = '' group_matrices.each do |k, v| matrices_list = [] @@ -392,9 +393,9 @@ def compare_group_matrices(group_matrices, options) matrices_list << matrix end if options[:no_print]

•  result_str += compare_matrixes(matrices_list, matrices_titles, k, options: options)
  

•  result_str += compare_matrixes(matrices_list, suites_list.shift, matrices_titles, k, options: options)
  

  else

•  print compare_matrixes(matrices_list, matrices_titles, k, options: options)
  

•  print compare_matrixes(matrices_list, suites_list.shift, matrices_titles, k, options: options)
  

  end end result_str

@@ -807,26 +808,40 @@ def get_theme(matrixes_values, matrixes_titles, theme) return THEMES[:none] end

-def get_all_result_str(matrixes_values, matrixes_titles, matrixes_number, theme) +def get_all_result_str(matrixes_values, suites_list, matrixes_titles, matrixes_number, theme) matrixes_titles ||= matrixes_number.times.to_a.map(&:to_s) theme = get_theme(matrixes_values, matrixes_titles, theme) return '' unless theme

• failure_str = get_result_str(matrixes_values[false].sort, matrixes_titles, false, theme)
• success_str = get_result_str(matrixes_values[true].sort, matrixes_titles, true, theme)

• failure_str = get_result_str(matrixes_values[false].sort, suites_list, matrixes_titles, false, theme)
• success_str = get_result_str(matrixes_values[true].sort, suites_list, matrixes_titles, true, theme) failure_str + success_str

end

-def get_result_str(values, matrixes_titles, success, theme) +def get_result_str(values, suites_list, matrixes_titles, success, theme) return '' if values.empty?

• suites_list.uniq!

Could we use "Set"?

ok

...

result_str = "\n\n\n" common_title, compare_title = get_title_name(success) result_str += get_header(matrixes_titles, success, common_title, compare_title)

• ranked_str = get_ranked_str(values, suites_list, success, theme)
• result_str += ranked_str
• result_str

+end

+def get_ranked_str(values, suites_list, success, theme)

• suite_str = ''
• common_str = '' values.each do |field, matrixes|

• result_str += get_values_str(matrixes, success, theme)
• result_str += get_field_str(field)
• result_str += "\n"

• row = get_values_str(matrixes, success, theme)
• row += get_field_str(field) + "\n"
• field_start_with_suite = suites_list.any? { |suite| field.start_with?(suite) }
• if field_start_with_suite

if suites_list.any? { |suite| field.start_with?(suite) }

"field_start_with_suite" was used only once.

good!

but i also think the variable name "field_start_with_suite" maybe more clear as condition of "if".

...

•  suite_str += row
  

• else

•  common_str += row
  

• end end

• result_str

• suite_str + common_str

end diff --git a/lib/matrix2.rb b/lib/matrix2.rb index 26f07fa..479d280 100644 --- a/lib/matrix2.rb +++ b/lib/matrix2.rb @@ -76,7 +76,9 @@ end # } def create_matrix(job_list) matrix = {}

• suites = [] job_list.each do |job|
• suites << job['suite'] if job['suite'] stats = job['stats'] next unless stats

@@ -89,7 +91,7 @@ def create_matrix(job_list) matrix.each_value do |value| samples_fill_missing_zeros(value, col_size) end

• matrix

• return matrix, suites

end

# input: query results from es_query @@ -110,15 +112,20 @@ end # ... # } def combine_group_query_data(query_data, dims)

• suites_list = [] job_list = query_data['hits']['hits'] groups = auto_group(job_list, dims) groups.each do |group_key, value|
• suite_list = [] value.each do |dimension_key, jobs|

•  groups[group_key][dimension_key] = create_matrix(jobs)
  

•  groups[group_key][dimension_key], suites = create_matrix(jobs)
  

•  suite_list.concat(suites)
  

  end groups.delete(group_key) if value.size < 2
• suites_list << suite_list unless value.size < 2

if value.size < 2 groups.delete(group_key) next end suites_list << suite_list

good!

btw, maybe it's more efficient to keep judgement of value.size on inner top of 'groups.each ...', like below:

suites_list = [] groups.each do |group_key, value| if value.size < 2 groups.delete[group_key] next end suite_list = [] value.each do |dimension_key, jobs| groups[group_key][dimension_key], suites = create_matrix(jobs) suite_list.concat(suites) end suites_list << suite_list end

Thanks, Kaiyi

Reduce repetitive logic operations.

Thanks, Zhang Yuhang

...

end

• groups

• return groups, suites_list

end

# input: @@ -139,7 +146,7 @@ def combine_group_jobs_list(query_data, groups_params, dimensions, metrics) groups = auto_group_by_template(job_list, groups_params, dimensions, metrics) groups.each do |group_key, dims| dims.each do |dim_key, jobs|

•  groups[group_key][dim_key] = create_matrix(jobs)
  

•  groups[group_key][dim_key], = create_matrix(jobs)
  

  end end

-- 2.23.0