feat: Use IEnumerable in WriteApi to eliminate unnecessary memory allocations #606
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
birdalicious
changed the title
bednar
changed the title
bednar
requested changes
Jan 18, 2024
There was a problem hiding this comment.
@birdalicious thanks for your PR 👍
Please satisfy our Checklist after that we are be able to approve this PR:
birdalicious
force-pushed
the
ienumerable-writeapi
branch
2 times, most recently
from
df800f2 to
29eed6e
Compare
|
What do I need to run |
|
You can start pre-configured InfluxDB instances in a Docker environment using the |
birdalicious
force-pushed
the
ienumerable-writeapi
branch
from
29eed6e to
9d8b7ff
Compare
bednar
requested changes
Feb 1, 2024
| @@ -220,7 +220,7 @@ public class WriteApiAsync : IWriteApiAsync | |||
| public Task WriteRecordsAsync(string[] records, WritePrecision precision = WritePrecision.Ns, | |||
| string bucket = null, string org = null, CancellationToken cancellationToken = default) | |||
| { | |||
| return WriteRecordsAsync(records.ToList(), precision, bucket, org, cancellationToken); | |||
| return WriteRecordsAsync(records, precision, bucket, org, cancellationToken); | |||
There was a problem hiding this comment.
This causes infinity recursive, please change to:
Suggested change
| return WriteRecordsAsync(records, precision, bucket, org, cancellationToken); | |
| return WriteRecordsAsync((IEnumerable<string>)records, precision, bucket, org, cancellationToken); |
| @@ -294,7 +293,7 @@ await WriteData(options.OrganizationId, options.Bucket, grouped.Key, groupedPoin | |||
| public Task WritePointsAsync(PointData[] points, string bucket = null, string org = null, | |||
| CancellationToken cancellationToken = default) | |||
| { | |||
| return WritePointsAsync(points.ToList(), bucket, org, cancellationToken); | |||
| return WritePointsAsync(points, bucket, org, cancellationToken); | |||
There was a problem hiding this comment.
This causes infinity recursive, please change to:
Suggested change
| return WritePointsAsync(points, bucket, org, cancellationToken); | |
| return WritePointsAsync((IEnumerable<PointData>)points, bucket, org, cancellationToken); |
| @@ -380,7 +379,7 @@ await WriteData(options.OrganizationId, options.Bucket, grouped.Key, groupedPoin | |||
| public Task WriteMeasurementsAsync<TM>(TM[] measurements, WritePrecision precision = WritePrecision.Ns, | |||
| string bucket = null, string org = null, CancellationToken cancellationToken = default) | |||
| { | |||
| return WriteMeasurementsAsync(measurements.ToList(), precision, bucket, org, cancellationToken); | |||
| return WriteMeasurementsAsync(measurements, precision, bucket, org, cancellationToken); | |||
There was a problem hiding this comment.
This causes infinity recursive, please change to:
Suggested change
| return WriteMeasurementsAsync(measurements, precision, bucket, org, cancellationToken); | |
| return WriteMeasurementsAsync((IEnumerable<TM>)measurements, precision, bucket, org, cancellationToken); |
|
This PR has been closed because it has not had recent activity. Please reopen if this PR is still important to you and you want to continue with them. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed Changes
Change WriteApi to use IEnumerable for collection arguments instead of List.
Change
WriteRecords(List<string> records)toWriteRecords(IEnumerable<string> records)Calling ToList on a collection will incur a memory allocation as the data is copied to the new list. Making the argument IEnumerable means ToList doesn't need to be called and so no extra memory allocation.
Internally this will eliminate the unnecessary memory allocation in
WriteRecords(string[] record)call to ToList, and client code also won't have to make the allocation if their data isn't already in a List## ChecklistChecklist
dotnet testcompletes successfully