performance: Avoid Recipient lookup for stream messages.

All the fields of a stream's recipient object can
be inferred from the Stream, so we just make a local
object.  Django will create a Message object without
checking that the child Recipient object has been
saved.  If that behavior changes in some upgrade,
we should see some pretty obvious symptom, including
query counts changing.

Tweaked by tabbott to add a longer explanatory comment, and delete a
useless old comment.
This commit is contained in:
Steve Howell 2020-10-19 19:49:36 +00:00 committed by Tim Abbott
parent 7bbcc2ac96
commit 85ed6f332a
4 changed files with 17 additions and 6 deletions

View File

@ -2335,9 +2335,20 @@ def check_message(sender: UserProfile, client: Client, addressee: Addressee,
stream = addressee.stream() stream = addressee.stream()
assert stream is not None assert stream is not None
recipient = stream.recipient # To save a database round trip, we construct the Recipient
# object for the Stream rather than fetching it from the
# This will raise JsonableError if there are problems. # database using the stream.recipient foreign key.
#
# This is simpler than ensuring that code paths that fetch a
# Stream that will be used for sending a message have a
# `select_related("recipient"), which would also needlessly
# expand Stream objects in memory (all the fields of Recipient
# are already known given the Stream object).
recipient = Recipient(
id=stream.recipient_id,
type_id=stream.id,
type=Recipient.STREAM,
)
if sender.bot_type != sender.OUTGOING_WEBHOOK_BOT: if sender.bot_type != sender.OUTGOING_WEBHOOK_BOT:
access_stream_for_send_message( access_stream_for_send_message(

View File

@ -963,7 +963,7 @@ class EditMessageTest(ZulipTestCase):
'propagate_mode': 'change_all', 'propagate_mode': 'change_all',
'topic': 'new topic', 'topic': 'new topic',
}) })
self.assertEqual(len(queries), 51) self.assertEqual(len(queries), 50)
messages = get_topic_messages(user_profile, old_stream, "test") messages = get_topic_messages(user_profile, old_stream, "test")
self.assertEqual(len(messages), 1) self.assertEqual(len(messages), 1)

View File

@ -1194,7 +1194,7 @@ class StreamMessagesTest(ZulipTestCase):
body=content, body=content,
) )
self.assert_length(queries, 14) self.assert_length(queries, 13)
def test_stream_message_dict(self) -> None: def test_stream_message_dict(self) -> None:
user_profile = self.example_user('iago') user_profile = self.example_user('iago')

View File

@ -3838,7 +3838,7 @@ class SubscriptionAPITest(ZulipTestCase):
principals=orjson.dumps([user1.id, user2.id]).decode(), principals=orjson.dumps([user1.id, user2.id]).decode(),
), ),
) )
self.assert_length(queries, 49) self.assert_length(queries, 48)
class GetStreamsTest(ZulipTestCase): class GetStreamsTest(ZulipTestCase):
def test_streams_api_for_bot_owners(self) -> None: def test_streams_api_for_bot_owners(self) -> None: