Significant performance boost: only write to the writer schema cache once.

[fimmtiu]

We noticed a performance problem with confluent-kafka-python while profiling our app: _get_decoder_func was being called a small number of times and took up a tiny amount of time, but _get_encoder_func seemed as if it was being called once per message and was taking up about 10% of our total runtime. This one-line patch gave us a 9% speed boost.

What's happening is that we have a cache for the encoder functions, but we're not actually using it in the encode_record_with_schema method:

confluent-kafka-python/confluent_kafka/avro/serializer/message_serializer.py

Line 113 in 50e09cf

self.id_to_writers[schema_id] = self._get_encoder_func(schema)

Every time that method is called, it will set a new entry in the id_to_writers cache, even if an identical one already exists. The fix is trivial: we should actually use the cache and not set new entries if there's already one in there.

[ghost]

It looks like @fimmtiu hasn't signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence.
Wikipedia

You can read and sign our full Contributor License Agreement here.

Once you've signed reply with [clabot:check] to prove it.

Appreciation of efforts,

clabot

[fimmtiu]

[clabot:check]

[ghost]

@confluentinc It looks like @fimmtiu just signed our Contributor License Agreement. 👍

Always at your service,

clabot

[fimmtiu]

@edenhill / @mhowlett : Any thoughts on this? It's a pretty trivial change with a nice benefit, and we've been using it in production for a few weeks now. (The CI failures seem unrelated to the actual code in the PR.)

[rnpridgeon]

Good find that is rather unfortunate.

Do you mind adding a quick test demonstrating the cache actually working. This will help to catch regressions in the future.

[fimmtiu]

@rnpridgeon Done! Thanks for the suggestion.

[AndreiLieb]

It seems to me that this can be fixed simply by removing line 113. Line 113 is not needed as encode_record_with_schema delegates the work to encode_record_with_schema_id where this logic is already present.

[mhowlett]

well pointed out @AndreiLieb . since the code does the job as is, and it'll be depreciated soon, just going with what's there to avoid mucking about. merged via #1057