[LNT] Python 3 support: adapt to dict method returning views

[thopre]

Summary

Adapt calls to dictionary's keys(), items() and values() to them
returning a view in Python 3 when necessary, undoing unneeded changes
in lnt/testing/__init__.py. Also replace calls to their iter relatives
to use iter() instead and calls to the their views relatives to use the
plains .keys(), .items() and .values(). This was produced by running
futurize's stage2 lib2to3.fixes.fix_dict with manual intervention to
clean up code and remove unnecessary list() wrapping.

[hubert.reinterpretcast]

examples/run_to_csv.py

10

I believe this use of next() is one of the ones that need to be replaced in the style of D67812. This file seems to be unmaintained and untested. None of the *.json files in the tests/ directory seems to match the format this script expects. I would suggest deleting this file.

15

I believe the change on this line is unnecessary.

lnt/lnttool/admin.py

32 ↗

(On Diff #220972)

I believe the change on this line is unnecessary.

50 ↗

(On Diff #220972)

I believe the change on this line is unnecessary. I'm not marking further instances, but we generally shouldn't form a list just to iterate.

lnt/server/db/rules/rule_update_fixed_regressions.py

78 ↗

(On Diff #220972)

I think this creates a list and then creates a sorted copy. I am inclined to leave the line unchanged (and for similar instances elsewhere in this patch).

lnt/server/ui/views.py

958

Is this use of sort something that would be considered within the scope of D67809?

1018

This also seems to be used just to iterate through once.

1322

Another sorted over a fresh list.

lnt/testing/profile/profilev2impl.py

273

keys.extend(f['counters'].keys())?

318

I guess the sorted() should go in place of list() where the latter is added a few lines up.

lnt/tests/nt.py

1484

Another call to sort. Maybe use something like the following instead?

sorted(filter(lambda bench: bench.is_rerunable(),
              collated_results.values()),
       key=lambda bench: bench.name)

1489

len(collated_results)

[thopre]

• Remove all wrapping into list that are unecessary due to views being iterable
• Improve uses of views to avoid introducing a call to list() when possible, as per Hubert

[thopre]

examples/run_to_csv.py

10

I prefer to apply the change for now and remove it or fix it in a later patch if that's ok. Strange the .next logic didn't detect this case. I've double checked the whole codebase for a call to .next() and didn't find any in python files.

lnt/lnttool/admin.py

50 ↗

(On Diff #220972)

Is there no multithreading going on in LNT?

lnt/server/ui/views.py

958

I think the 2to3 fix_idioms only fires if sort is called without parameter. Presumably if the call to sort takes some parameters that might make the folding both lines into sorted too long and thus be difficult to read. I think in this specific case it would indeed be harder to read so I'm enclined to leave it like this.

1497

testsuites is only used for iterating below so I removed the list here.

lnt/testing/__init__.py

260

I'm removing the list here which I added when committing https://reviews.llvm.org/D65751

lnt/tests/nt.py

1484

As mentioned, the 2to3 sort change seems to skip this sort of folding intentionally, presumably to avoid a multiline expression. I'm therefore sticking to having filter and sort as separate steps. I did however changed to use the filter builtin as it makes the intent clearer IMHO.

lnt/util/multidict.py

19–26 ↗

(On Diff #220972)

All uses of these methods already assume this returns views and have been converted to use list if necessary. Ultimately we should remove that file and rely on the pypi package IMHO.

[hubert.reinterpretcast]

examples/run_to_csv.py

10

Okay.

lnt/lnttool/admin.py

50 ↗

(On Diff #220972)

config is local to the function, so I don't think it is being concurrently modified here.

lnt/server/ui/views.py

958

Okay.

lnt/tests/nt.py

1489

I'm not seeing the change. len applied to the dictionary directly works.

[thopre]

Apply len to dictionary rather than dictionary values

lnt/lnttool/admin.py

50 ↗

(On Diff #220972)

I meant in general, I don't know enough the code to make a good judgement call. However IIRW iterating on a views while it is being updated raises an assertion so in the worse case it should be quite obvious what is going on.

lnt/tests/nt.py

1489

Oh my bad, missed your point.

[hubert.reinterpretcast]

LGTM.

examples/run_to_csv.py

 """Convert the JSON run file passed on stdin to a csv"""
 from __future__ import print_function
 import json
 
 import sys
 
 
 data = json.load(sys.stdin)
 
-titles = data['tests'].itervalues().next().keys()
+titles = list(next(iter(data['tests'].values())).keys())
 titles.insert(0, titles.pop(titles.index("name")))
 
 print(", ".join(titles))
 
 for i, result in data['tests'].items():
 
     for title in titles:
         print(result[title], end=' ')
         print(", ", end=' ')
     print()

lnt/server/config.py

def get_database(self, name):
         db_entry = self.databases.get(name)
         if db_entry is None:
             return None
 
         return lnt.server.db.v4db.V4DB(db_entry.path, self,
                                        db_entry.baseline_revision)
 
     def get_database_names(self):
-        return self.databases.keys()
+        return list(self.databases.keys())

lnt/server/reporting/summaryreport.py

def _build_data_table(self):
                 # Preload the run parameters.
                 run_parameters_map = dict((r.id, r.parameters)
                                           for r in ts_runs)
 
                 # Load all the samples for all runs we are interested in.
                 columns = [ts.Sample.run_id, ts.Sample.test_id]
                 columns.extend(f.column for f in ts.sample_fields)
                 samples = session.query(*columns).filter(
-                    ts.Sample.run_id.in_(run_id_map.keys()))
+                    ts.Sample.run_id.in_(list(run_id_map.keys())))
                 for sample in samples:
                     run = run_id_map[sample[0]]
                     datapoints = list()
                     for key, value in get_datapoints_for_sample(ts, sample):
                         items = self.data_table.get(key)
                         if items is None:
                             items = [[]
                                      for _ in self.report_orders]

lnt/server/ui/regression_views.py

reg_inds = session.query(ts.RegressionIndicator) \
         .all()
     return reg_inds, regressions
 
 
 class EditRegressionForm(Form):
     title = StringField(u'Title', validators=[DataRequired()])
     bug = StringField(u'Bug', validators=[DataRequired()])
     field_changes = MultiCheckboxField("Changes", coerce=int)
-    choices = RegressionState.names.items()
+    choices = list(RegressionState.names.items())
     state = SelectField(u'State', choices=choices)
     edit_state = HiddenField(u'EditState', validators=[DataRequired()])
 
 
 def name(cls):
     """Get a nice name for this object."""
     return cls.__class__.__name__
 

lnt/server/ui/views.py

for i, (machine, test, field, field_index) in enumerate(graph_parameters):
 
         # Unless all samples requested, filter out failing tests.
         if not show_failures:
             if field.status_field:
                 q = q.filter((field.status_field.column == PASS) |
                              (field.status_field.column.is_(None)))
 
         # Aggregate by revision.
-        data = multidict.multidict((rev, (val, date, run_id))
-                                   for val, rev, date, run_id in q).items()
+        data = list(multidict.multidict((rev, (val, date, run_id))
+                                        for val, rev, date, run_id in q)
+                    .items())
 
         data.sort(key=lambda sample: convert_revision(sample[0], cache=revision_cache))
 
         graph_datum.append((test.name, data, col, field, url, machine))
 
         # Get baselines for this line
         num_baselines = len(baseline_parameters)
         for baseline_id, (baseline, baseline_title) in \
                 enumerate(baseline_parameters):
             q_baseline = session.query(field.column,
if mean_parameter:
                           sqlalchemy.sql.func.min(ts.Run.start_time)) \
                    .join(ts.Run).join(ts.Order).join(ts.Test) \
                    .filter(ts.Run.machine_id == machine.id) \
                    .filter(field.column.isnot(None)) \
                    .group_by(ts.Order.llvm_project_revision, ts.Test)
 
         # Calculate geomean of each revision.
         data = multidict.multidict(
                     ((rev, date), val) for val, rev, date in q).items()
         data = [(rev,
                  [(lnt.server.reporting.analysis.calc_geomean(vals), date)])
                 for ((rev, date), vals) in data]
 
         # Sort data points according to revision number.
         data.sort(key=lambda sample: convert_revision(sample[0]))
 
         graph_datum.append((test_name, data, col, field, None, machine))
recent_runs = session.query(ts.Run) \
         .all()
 
     # Aggregate the runs by machine.
     recent_runs_by_machine = multidict.multidict()
     for run in recent_runs:
         recent_runs_by_machine[run.machine] = run
 
     # Get a sorted list of recent machines.
     recent_machines = sorted(recent_runs_by_machine.keys(),
                              key=lambda m: m.name)
 
     # We use periods in our machine names. css does not like this
     # since it uses periods to demark classes. Thus we convert periods
     # in the names of our machines to dashes for use in css. It is
     # also convenient for our computations in the jinja page to have
     # access to
     def get_machine_keys(m):
def v4_summary_report_ui():
     else:
         config = {
             "machine_names": [],
             "orders": [],
             "machine_patterns": [],
         }
 
     # Get the list of available test suites.
     testsuites = request.get_db().testsuite.values()
 
     # Gather the list of all run orders and all machines.
     def to_key(name):
         first = name.split('.', 1)[0]
         if first.isdigit():
             return (int(first), name)
         return (first, name)
     all_machines = set()

lnt/testing/__init__.py

class Test:
     have the same test name but different info paramaters.
     """
 
     def __init__(self, name, samples, info={}, report_version=2):
         self.name = name
         self.samples = samples
         self.info = dict()
         # Convert keys/values that are not json encodable to strings.
-        for key, value in list(info.items()):
+        for key, value in info.items():
             key = str(key)
             value = str(value)
             self.info[key] = value
         self.report_version = report_version
         self.check()
 
     def check(self):
         """Check object members are adequate to generate an LNT json

lnt/testing/profile/profilev2impl.py

def deserialize(self, fobj):
             self.idx_to_name[i] = readString(fobj)
         self.name_to_idx = {v: k
                             for k, v
                             in self.idx_to_name.items()}
 
     def upgrade(self, impl):
         self.idx_to_name = {}
 
-        keys = impl.getTopLevelCounters().keys()
+        keys = list(impl.getTopLevelCounters().keys())
         for f in impl.getFunctions().values():
-            keys += f['counters'].keys()
+            keys.extend(f['counters'].keys())
         keys = sorted(set(keys))
 
         self.idx_to_name = {k: v for k, v in enumerate(keys)}
         self.name_to_idx = {v: k for k, v in enumerate(keys)}
 
 
 class TopLevelCounters(Section):
     def __init__(self, counter_name_pool):
         self.counter_name_pool = counter_name_pool
 
class LineCounters(CompressedSection):
 
     def serialize(self, fobj):
         assert self.impl
 
         self.function_offsets = {}
         start = fobj.tell()
         for fname, f in sorted(self.impl.getFunctions().items()):
             self.function_offsets[fname] = fobj.tell() - start
-            all_counters = f['counters'].keys()
+            all_counters = sorted(f['counters'].keys())
             for counters, address, text in self.impl.getCodeForFunction(fname):
-                for k in sorted(all_counters):
+                for k in all_counters:
                     writeFloat(fobj, counters.get(k, 0))
 
     def deserialize(self, fobj):
         self.data = fobj.read()
 
     def upgrade(self, impl):
         self.impl = impl
         self.function_offsets = {}
class Functions(Section):
 
     def upgrade(self, impl):
         self.impl = impl
         self.functions = self.impl.getFunctions()
 
     def getCodeForFunction(self, fname):
         f = self.functions[fname]
         counter_gen = self.line_counters \
-            .extractForFunction(fname, f['counters'].keys())
+            .extractForFunction(fname, list(f['counters'].keys()))
         address_gen = self.line_addresses.extractForFunction(fname)
         text_gen = self.line_text.extractForFunction(fname)
         for n in xrange(f['length']):
             yield (next(counter_gen), next(address_gen), next(text_gen))
 
     def copy(self, counter_name_pool, line_counters,
              line_addresses, line_text):
         new = copy.copy(self)

lnt/tests/nt.py

for full_name, results_status, perf_status in server_results:
         else:
             assert False, "Unexpected server result type." + test_type
         collated_results[test_name] = new_entry
 
     # Double check that all values are there for all tests.
     for test in collated_results.values():
         test.check()
 
-    rerunable_benches = [x for x in collated_results.values()
-                         if x.is_rerunable()]
+    rerunable_benches = list(filter(lambda bench: bench.is_rerunable(),
+                                    collated_results.values()))
     rerunable_benches.sort(key=lambda x: x.name)
     # Now lets do the reruns.
     rerun_results = []
     summary = "Rerunning {} of {} benchmarks."
     logger.info(summary.format(len(rerunable_benches),
                                len(collated_results.values())))
 
     for i, bench in enumerate(rerunable_benches):
         logger.info("Rerunning: {} [{}/{}]".format(bench.name,
                                                    i + 1,
                                                    len(rerunable_benches)))
 
         fresh_samples, t_no_errors = rerun_test(config,
                                                 bench.name, NUMBER_OF_RERUNS)

tests/server/ui/V4Pages.py

def dump_html(html_string):
     for linenr, line in enumerate(html_string.split('\n')):
         print("%4d:%s" % (linenr + 1, line))
 
 
 def get_xml_tree(html_string):
     try:
         parser = ET.XMLParser()
         parser.parser.UseForeignDTD(True)
-        parser.entity.update((x, unichr(i)) for x, i in name2codepoint.iteritems())
+        parser.entity.update((x, unichr(i)) for x, i in name2codepoint.items())
         tree = ET.fromstring(html_string, parser=parser)
     except:  # noqa FIXME: figure out what we expect this to throw.
         dump_html(html_string)
         raise
     return tree
 
 
 def find_table_by_thead_content(tree, table_head_contents):

tests/server/ui/test_roundtrip.py

def _compare_results(self, after_submit_run, before_submit_run):
         an_id = 1234567
         before_submit_run['run']['id'] = an_id
         before_submit_run['machine']['id'] = an_id
         after_submit_run['run']['id'] = an_id
         after_submit_run['machine']['id'] = an_id
         before_submit_run['run']['order_id'] = an_id
         after_submit_run['run']['order_id'] = an_id
 
-        self.assertEqual(before_submit_run.keys(), after_submit_run.keys())
+        self.assertEqual(list(before_submit_run.keys()), list(after_submit_run.keys()))
         # Machine and run will be dicts, compare them directly.
         for k in ['machine', 'run']:
             self.assertEqual(before_submit_run[k], after_submit_run[k])
         # The order of the tests might have changed, so sort before they are compared.
         before_submit_tests = sorted(before_submit_run['tests'])
         after_submit_tests = sorted(after_submit_run['tests'])
         for i, _ in enumerate(before_submit_tests):
             before_submit_tests[i]['run_id'] = 1234