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

Memory Spike getting QR Code #55

Closed
eramosr16 opened this issue Aug 27, 2020 · 14 comments
Closed

Memory Spike getting QR Code #55

eramosr16 opened this issue Aug 27, 2020 · 14 comments
Assignees

Comments

@eramosr16
Copy link

eramosr16 commented Aug 27, 2020

I came across this issue creating the QR Code image inside a docker container with limited resources in RAM, the container starts with 140mb of ram and once I call the endpoint to create the image it goes up to 1.7 GB of Ram just after calling the method. Is there a way we can reduce this.

Before:
image

After:
image

This was using Aspnetcore 2.2 and GoogleAuthenticator v2.0.1
Method that generated the spike: GenerateSetupCode

@flytzen
Copy link
Collaborator

flytzen commented Aug 27, 2020

Thank you for reporting this. From memory, I think this is a call-out to another lib. Let us have a look...

@flytzen flytzen self-assigned this Aug 27, 2020
@ahwm
Copy link
Collaborator

ahwm commented Aug 27, 2020

You are correct, @flytzen - it's using QRCoder 1.3.5 for the QR generation. It looks like there's a newer version. Might be worth updating that library in your project and see if that addresses the issue.

Looks like 1.3.7 addressed memory issues

@eramosr16
Copy link
Author

I appreciate your quick response guys, keep up the good work.
If you don't mind let me know when this is out to update my references over here.

@ahwm ahwm linked a pull request Aug 27, 2020 that will close this issue
@ahwm ahwm removed a link to a pull request Aug 27, 2020
@flytzen
Copy link
Collaborator

flytzen commented Aug 27, 2020

@BrandonPotter v2.0.2 is ready for you to approve in AzDO

@eramosr16
Copy link
Author

eramosr16 commented Aug 27, 2020

Hey guys, while I was waiting for your release I went ahead and clone the repo and built the package myself. I tested again and it seems the library upgrade didn't fix the issue:

image

After finishing the call the memory went down to:

image

Just to stay there, perhaps the issue is on the dependency.
Replacing it with a more efficient one might be a possible solution.

As a temporary solution I had to increase the Pod memory resources to 4GB since I found that a 2nd call to function with 2Gb of memory still crash the pod.

Have a great day.

@flytzen
Copy link
Collaborator

flytzen commented Aug 27, 2020

Ah that sucks. Okay, we'll dig into it some more. @ahwm I'll try to do some profiling and stuff to see if we can narrow it down.

@ahwm
Copy link
Collaborator

ahwm commented Aug 27, 2020

Sounds good. Let me know what you're able to find. I was toying with the idea of setting up a basic API that would allow the QR creation to be offloaded. If it's a completely open source web service that doesn't store anything and uses POST so that data in URLs isn't logged (the primary security concern for TOTP QR generation) then it should be all good. In theory.

In the meantime, I'll see if there are any other libraries out there that would be able to generate them.

@ahwm
Copy link
Collaborator

ahwm commented Aug 27, 2020

https://github.com/BrandonPotter/GoogleAuthenticator/tree/Net.Codecrete.QrCodeGenerator Uses a different library.

I suspect the memory usage might actually be related to the Bitmap and MemoryStream objects for creating/storing the QR code (as opposed to the actual generation of such from the other library). I'm not sure if there's anyway to rewrite it to reduce memory usage.

@eramosr16 Can you provide a code snippet of your GenerateSetupCode() - especially what you are passing as the QRPixelsPerModule parameter? Is it relatively high (creating a large image)?

@eramosr16
Copy link
Author

This is the endpoint, I had to hide sensitive information, sorry about that.

image

@ahwm
Copy link
Collaborator

ahwm commented Aug 27, 2020

You're totally fine. I was most interested in that last parameter. I think that's the issue. When I use 250 I get a gigantic (14,250x14,250 pixels) image that is 1.6 MB.
image

When I use 3 in that parameter I get this:
image

@ahwm ahwm pinned this issue Aug 27, 2020
@ahwm
Copy link
Collaborator

ahwm commented Aug 27, 2020

Similar to #47.

I guess we could try to make it more clear what a "module" is.

@eramosr16
Copy link
Author

You're totally fine. I was most interested in that last parameter. I think that's the issue. When I use 250 I get a gigantic (14,250x14,250 pixels) image that is 1.6 MB.
image

When I use 3 in that parameter I get this:
image

I agree with you, I'll update to a lower value and test, but do you think that will also justify the jump from 140 Mb to 1.7 Gb of Memory?.

@ahwm
Copy link
Collaborator

ahwm commented Aug 28, 2020

I don’t know. It’s worth a try and see how big a difference it makes.

@eramosr16
Copy link
Author

@ahwm I did a test reducing the number from 250 to 50 and the difference was huge it went from 1.7Gb to 300Mb, it seems the issue is there, I went and download the QR library Code and they have a double loop when they go and render the image, which will increase depending on the amount of pixels, offsets and another bunch of variables. They also had a ticket to fix that, I realize that library also had a very poor support this year checking the activity the last update was on April.

I think you can close this ticket since is not related with you directly, I appreciate all your effort to fix this and your feedback.
Keep up the good work.
Best.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants