Skip to content
This repository was archived by the owner on Oct 29, 2024. It is now read-only.

improved make_lines efficiency #433

Merged
merged 1 commit into from
Apr 5, 2017
Merged

improved make_lines efficiency #433

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 12 additions & 16 deletions influxdb/line_protocol.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,12 @@
from __future__ import print_function
from __future__ import unicode_literals

from copy import copy
from datetime import datetime
from numbers import Integral

from pytz import UTC
from dateutil.parser import parse
from six import binary_type, text_type, integer_types, PY2
from six import iteritems, binary_type, text_type, integer_types, PY2

EPOCH = UTC.localize(datetime.utcfromtimestamp(0))

Expand Down Expand Up @@ -108,7 +107,7 @@ def make_lines(data, precision=None):
matching the line protocol introduced in InfluxDB 0.9.0.
"""
lines = []
static_tags = data.get('tags', None)
static_tags = data.get('tags')
for point in data['points']:
elements = []

Expand All @@ -119,32 +118,29 @@ def make_lines(data, precision=None):
key_values = [measurement]

# add tags
if static_tags is None:
tags = point.get('tags', {})
if static_tags:
tags = dict(static_tags) # make a copy, since we'll modify
tags.update(point.get('tags') or {})
else:
tags = copy(static_tags)
tags.update(point.get('tags', {}))
tags = point.get('tags') or {}

# tags should be sorted client-side to take load off server
for tag_key in sorted(tags.keys()):
for tag_key, tag_value in sorted(iteritems(tags)):
key = _escape_tag(tag_key)
value = _escape_tag(tags[tag_key])
value = _escape_tag(tag_value)

if key != '' and value != '':
key_values.append("{key}={value}".format(key=key, value=value))
key_values.append(key + "=" + value)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe it's better to use string formatting methods rather than appending strings via the + operator, can you please restore that behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@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?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I stand corrected, that's my fault for not running some tests 👍. Looks like a good change.

Copy link

@pkittenis pkittenis Apr 4, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link

@pkittenis pkittenis Apr 4, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

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_values = ','.join(key_values)
elements.append(key_values)

# add fields
field_values = []
for field_key in sorted(point['fields'].keys()):
for field_key, field_value in sorted(iteritems(point['fields'])):
key = _escape_tag(field_key)
value = _escape_value(point['fields'][field_key])
value = _escape_value(field_value)
if key != '' and value != '':
field_values.append("{key}={value}".format(
key=key,
value=value
))
field_values.append(key + "=" + value)
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto above regarding concatenating strings

field_values = ','.join(field_values)
elements.append(field_values)

Expand Down