We have a memory leak when a new image is uploaded and it is more apparent when there are already several images uploaded already as a question response. We used memory tools and tracked the issue down to renderPreview() function. Here is how our upload mechanism is laid out:
- User clicks on upload files click
- User selects an image
- The image is stored on our servers as base64
- If the server request is successful the uploadSuccessCallBack() is called
- This will call renderPreview() function. When the render preview function is called, the memory usage goes up and doesn't come back until we restart the app.
We believe the renderer keeps a copy of the base64 images in memory and doesn't clean.
6. In uploadSuccessCallBack() we concatenate the new image (as base64)to this.question.value. (i.e.
this.question.value = (this.question.value || []).concat(
{
name: r.fileName,
type: r.type,
content: r.content,
fileData: r.fileData
});
)
This will call renderPreview() function. When the render preview function is called, the memory usage goes up and doesn't come back until we restart the app.
We believe the renderer keeps a copy of the base64 images in memory and doesn't clean. We wonder if there is a better/more efficient way to update the question instead of using .concat().
Hello,
Probably your question is rather related to SurveyJS Library than to Dashbload product. Could you please share a minimal reproducible example illustrating the issue so we could investigate it on our side and fix the issue?
Thanks, Serge
SurveyJS Team
After investigation I found out that the leak happens in getUnbindValue() method of helpers.ts file in SurveyJS.
So in our case we have a Survey that already has 30-40 images. Each of these images are stored and mounted as Base64. Since we don't store thumbnails the images are mounted as full size ( So ~100 mb for 30 images). When a new image is uploaded, the new image is returned in uploadSuccessCallback(). Then getUnbindValue () function is called through the below process.
upladSuccessCallBack --> setNewValue() --> setValue() --> notifyQuestionOnValueChnaged() --> onSurveyValueChanged() --> updateDisplayValue () --> getDisplayValue() --> createValueCopy() --> getUnbindValue()
In this scenario getUnbindValue() is called with and array of 30 images each of the array element is an object of the following type (Also screen shot attached shows this type):
{content: string, fileData: string (base64value), name: string, size: number, type: string}
Here is the implementation of getUnbindValue().
public static getUnbindValue(value: any): any { if (!!value && value instanceof Object && !(value instanceof Date)) { //do not return the same object instance!!! return JSON.parse(JSON.stringify(value)); } return value; }
I verified that calling JSON.parse(JSON.stringify(value)) on a large array causes the memory leak. What is interesting to note is the argument passed to this function is an Array but it is still considered as an object and the statement of
if (!!value && value instanceof Object && !(value instanceof Date))
is resolved to true. I wonder if we should add another check for Arrays, so if the argument is an array, the above if statement is not resolved to true?
Hello,
getUnbindValue
should return another instance of an array. We did not expect to have an array of several Megabytes strings inside it.We will replace JSON parse/stringify functions in
getUnbindValue
function and come back to you.Thank you,
Andrew
SurveyJS Team
Hello Salar,
I have create the PR that should fix the issue.
We will merge it into master and include into new version that we will release next week. We release weekly.
It should solve the issue. Please let us know how it works.
Thank you,
Andrew
SurveyJS Team
Thank you!
You are very welcome!
Andrew
SurveyJS Team
Hey Andrew thank so much for putting in the fix. However, I am trying to understand what this function tries to achieve and what happens if we just return the value when we have an array instead of iterating through the array. BTW I left a comment in the above PR as well. Feel free to respond to either of them.
So instead of
public static getUnbindValue(value: any): any { if(Array.isArray(value)) { const res = []; for(let i = 0; i < value.length; i ++) { res.push(Helpers.getUnbindValue(value[i])); } return res; } if (!!value && value instanceof Object && !(value instanceof Date)) { const res: any = {}; const keys = Object.keys(value); keys.forEach(key => { const val = Helpers.getUnbindValue(value[key]); if(val !== undefined) { res[key] = val; } }); return res; } return value; }
We have something similar to :
public static getUnbindValue(value: any): any { if(Array.isArray(value)) { return value: } if (!!value && value instanceof Object && !(value instanceof Date)) { const res: any = {}; const keys = Object.keys(value); keys.forEach(key => { const val = Helpers.getUnbindValue(value[key]); if(val !== undefined) { res[key] = val; } }); return res; } return value; }
Hello Salar,
I have answered here.
Thank you,
Andrew
SurveyJS Team