Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Consistently use six's iteritems and wrap lazy keys/values in list() if they're not meant to be lazy #3307

Merged
merged 4 commits into from
May 31, 2018

Conversation

hawkowl
Copy link
Contributor

@hawkowl hawkowl commented May 30, 2018

No description provided.

Copy link
Member

@erikjohnston erikjohnston left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels like this is adding a lot of unnecessary list copying for python2, can we avoid that please?

levels_to_check.append(
(user, "users")
)

old_list = current_state.content.get("events", {})
new_list = event.content.get("events", {})
for ev_id in set(old_list.keys() + new_list.keys()):
for ev_id in set(list(old_list.keys()) + list(new_list.keys())):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're doing list(foo.keys()) can't we just do list(foo)? Rather than in py2 have the lists be copied twice

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, done.

@@ -146,7 +146,7 @@ def __contains__(self, field):
return field in self._event_dict

def items(self):
return self._event_dict.items()
return list(self._event_dict.items())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be nicer to do list(iteritems(...))?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's about twice as slow on Python 2, unfortunately:

~ $ python3 -m timeit "list(globals().items())"
1000000 loops, best of 3: 0.75 usec per loop
~ $ python -m timeit "list(globals().items())"
1000000 loops, best of 3: 0.67 usec per loop
~ $ python -m timeit "list(globals().iteritems())"
1000000 loops, best of 3: 1.02 usec per loop

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well that's fun :/

@erikjohnston erikjohnston assigned hawkowl and unassigned erikjohnston May 30, 2018
@hawkowl
Copy link
Contributor Author

hawkowl commented May 31, 2018

@erikjohnston I removed the instances where there was an extra list copy. There is a little bit of overhead with it on items or values, but I think it's negligible enough.

~ $ python -m timeit "list(globals().values())"
1000000 loops, best of 3: 0.298 usec per loop
~ $ python -m timeit "globals().values()"
10000000 loops, best of 3: 0.168 usec per loop
~ $ python -m timeit "list(globals().items())"
1000000 loops, best of 3: 0.665 usec per loop
~ $ python -m timeit "globals().items()"
1000000 loops, best of 3: 0.471 usec per loop

@hawkowl hawkowl merged commit c936a52 into develop May 31, 2018
@hawkowl hawkowl deleted the hawkowl/iteritems branch May 31, 2018 09:03
neilisfragile added a commit that referenced this pull request Jun 6, 2018
Changes in synapse v0.31.0 (2018-06-06)
======================================

Most notable change from v0.30.0 is to switch to python prometheus library to improve system
stats reporting. WARNING this changes a number of prometheus metrics in a
backwards-incompatible manner. For more details, see
`docs/metrics-howto.rst <docs/metrics-howto.rst#removal-of-deprecated-metrics--time-based-counters-becoming-histograms-in-0310>`_.

Bug Fixes:

* Fix metric documentation tables (PR #3341)
* Fix LaterGuage error handling (694968f)
* Fix replication metrics (b7e7fd2)

Changes in synapse v0.31.0-rc1 (2018-06-04)
==========================================

Features:

* Switch to the Python Prometheus library (PR #3256, #3274)
* Let users leave the server notice room after joining (PR #3287)

Changes:

* daily user type phone home stats (PR #3264)
* Use iter* methods for _filter_events_for_server (PR #3267)
* Docs on consent bits (PR #3268)
* Remove users from user directory on deactivate (PR #3277)
* Avoid sending consent notice to guest users (PR #3288)
* disable CPUMetrics if no /proc/self/stat (PR #3299)
* Add local and loopback IPv6 addresses to url_preview_ip_range_blacklist (PR #3312) Thanks to @thegcat!
* Consistently use six's iteritems and wrap lazy keys/values in list() if they're not meant to be lazy (PR #3307)
* Add private IPv6 addresses to example config for url preview blacklist (PR #3317) Thanks to @thegcat!
* Reduce stuck read-receipts: ignore depth when updating (PR #3318)
* Put python's logs into Trial when running unit tests (PR #3319)

Changes, python 3 migration:

* Replace some more comparisons with six (PR #3243) Thanks to @NotAFile!
* replace some iteritems with six (PR #3244) Thanks to @NotAFile!
* Add batch_iter to utils (PR #3245) Thanks to @NotAFile!
* use repr, not str (PR #3246) Thanks to @NotAFile!
* Misc Python3 fixes (PR #3247) Thanks to @NotAFile!
* Py3 storage/_base.py (PR #3278) Thanks to @NotAFile!
* more six iteritems (PR #3279) Thanks to @NotAFile!
* More Misc. py3 fixes (PR #3280) Thanks to @NotAFile!
* remaining isintance fixes (PR #3281) Thanks to @NotAFile!
* py3-ize state.py (PR #3283) Thanks to @NotAFile!
* extend tox testing for py3 to avoid regressions (PR #3302) Thanks to @krombel!
* use memoryview in py3 (PR #3303) Thanks to @NotAFile!

Bugs:

* Fix federation backfill bugs (PR #3261)
* federation: fix LaterGauge usage (PR #3328) Thanks to @intelfx!
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants