Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question: reduce memory usage as much as possible #31337

Closed
JanEggers opened this issue Jul 23, 2023 · 7 comments
Closed

Question: reduce memory usage as much as possible #31337

JanEggers opened this issue Jul 23, 2023 · 7 comments
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported

Comments

@JanEggers
Copy link

im trying to reduce the allocations in the hot path of our rest api. by basically manually json formatting the result as part of the linq i was able to avoid the allocation of the model object. currently experimenting with EF7. my result set is 10K rows.

When looking at the allocation performance profiler there seem to be some allocations because of async locals used by ef. but i found a way around that by disabling the concurrency check. ( .EnableThreadSafetyChecks(false))

what remains are the formatted strings and the values that are formatted into the string.

image

Is there any way to translate the string.format call to sql so I get back just the actual result I want without the need to execute string.format on the client? I also tried to use string interpolation but that seems to use an additional object array.

Include your code

var companiesAsJson = _context.Companies.Take(take).AsNoTracking()
        .Select(c => string.Format("{{\"Id\": {0}, \"Name\": \"{1}\"  }}", c.Id, c.Name));

var writer = Response.BodyWriter;
      var encoding = Encoding.UTF8;

      writer.Write(encoding.GetBytes("["));

      foreach (var company in companiesAsJson)
      {
          encoding.GetBytes(company, writer);
      }
      writer.Write(encoding.GetBytes("]"));

      await writer.FlushAsync();
@roji
Copy link
Member

roji commented Jul 23, 2023

@JanEggers yes, you should be able to use the string concatenation operator instead (e.g. .Select(c => "Id: " + c.Id + " Name: " + c.Name)).

I'm curious if you're seeing an actual performance difference by eliminating these allocations (this also includes EnableThreadSafetyChecks). I appreciate general attempts to e.g. get allocations down to zero, but in almost all real-world applications these shouldn't have any actual impact on the app's end-to-end throughput.

@JanEggers
Copy link
Author

with my current approach the api response time drops from 40ms to 20ms so i call that real gains. I will try your suggestion

@JanEggers
Copy link
Author

looking better :)

image

i suppose ony you guys can fix the char[] allocation in tdsparser?!

@roji
Copy link
Member

roji commented Jul 23, 2023

with my current approach the api response time drops from 40ms to 20ms so i call that real gains.

I'd be very surprised if something like EnableThreadSafetyChecks() or the transition from client-side string.Format to server-side string concatenation resulted in such a huge change; it's very likely that some other unrelated change you did is responsible for that. Mind you, I'm not trying to argue against performance optimizations, just to be very clear on which change produces which kind of gain, in order to save you a lot of micro-optimization work that won't affect your end-to-end perf in any way.

@roji
Copy link
Member

roji commented Jul 23, 2023

i suppose ony you guys can fix the char[] allocation in tdsparser?

You can try asking on the SqlClient repo, though I suspect that allocation may be necessary for it to do its work.

I'm going to go ahead and close this as there's no additional outstanding question, but feel free to post back with more info or questions.

@roji roji closed this as not planned Won't fix, can't repro, duplicate, stale Jul 23, 2023
@roji roji added the closed-no-further-action The issue is closed and no further action is planned. label Jul 23, 2023
@JanEggers
Copy link
Author

issue in sql client was already fixed just had to update :)

dotnet/SqlClient#1866

@roji roji closed this as not planned Won't fix, can't repro, duplicate, stale Jul 23, 2023
@Havunen
Copy link

Havunen commented Aug 12, 2023

The issue is not fixed yet, but its slightly better:
dotnet/SqlClient#2120

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported
Projects
None yet
Development

No branches or pull requests

3 participants