improved make_lines efficiency #433
Conversation
no change in functionality; only efficiency.
|
|
||
| if key != '' and value != '': | ||
| key_values.append("{key}={value}".format(key=key, value=value)) | ||
| key_values.append(key + "=" + value) |
There was a problem hiding this comment.
I believe it's better to use string formatting methods rather than appending strings via the + operator, can you please restore that behavior?
There was a problem hiding this comment.
@xginn8 Thanks for the review!
Better in what way? + is generally one of the fastest methods of concatenating short strings; conversely, format with keywords is one of the slowest.
Because key + "=" + value is short and readable, and because this code is in a frequently executed loop body, I saw no need to sacrifice performance--we can have both efficient and readable code. Thoughts?
There was a problem hiding this comment.
I stand corrected, that's my fault for not running some tests 👍. Looks like a good change.
There was a problem hiding this comment.
key_values.append("=".join((key, value))) would be faster[1] actually.
String concatenation creates temporary strings for each concatenation and will get slower as strings gets larger.
join OTOH is implemented in C and does the whole operation in one go.
[1] For large enough strings
There was a problem hiding this comment.
Speed wise would actually be best to replace the whole loop with a list comprehension:
key_values = ["=".join((key, _escape_tag(val)))
for key, val in sorted(iteritems(tags))
if key != "" and val != ""]
Cleaner and more readable to boot.
| key=key, | ||
| value=value | ||
| )) | ||
| field_values.append(key + "=" + value) |
There was a problem hiding this comment.
ditto above regarding concatenating strings
|
Thank you for the PR! Thanks to @xginn8 for reviewing :) |
This PR reduces some unneeded allocations and operations in
make_lines.We're using the influxdb client in our high throughput, low latency web service. We've profiled our service and found that
make_lineswas a bottleneck. Whenmake_linestakes a long time, it blocks other greenlets from servicing requests.No change in functionality; only efficiency.