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

[UWP][Memory] [Rendered Cards Leaking Memory] #3425

Closed
andycb opened this issue Sep 6, 2019 · 12 comments
Closed

[UWP][Memory] [Rendered Cards Leaking Memory] #3425

andycb opened this issue Sep 6, 2019 · 12 comments
Labels

Comments

@andycb
Copy link

andycb commented Sep 6, 2019

Platform

  • UWP

Version of SDK

AdaptiveCards.Redering.Uwp V1.2.2

Details

When redering an adaptive card, it appears that its memory is not released. The bellow code sample loads an adaptive card, displays it on the UI before removing it and forcing a garbage collection. After 50 itteraltions, an additional 90MB of memory is being used by the test app.

image

var renderer = new AdaptiveCardRenderer();
string card = "{   \"$schema\": \"http://adaptivecards.io/schemas/adaptive-card.json\",   \"type\": \"AdaptiveCard\",   \"version\": \"1.0\",   \"body\": [     {       \"type\": \"TextBlock\",       \"text\": \"Publish Adaptive Card schema\",       \"weight\": \"bolder\",       \"size\": \"medium\"     },     {       \"type\": \"ColumnSet\",       \"columns\": [         {           \"type\": \"Column\",           \"width\": \"auto\",           \"items\": [             {               \"type\": \"Image\",               \"url\": \"https://pbs.twimg.com/profile_images/3647943215/d7f12830b3c17a5a9e4afcc370e3a37e_400x400.jpeg\",               \"size\": \"small\",               \"style\": \"person\"             }           ]         },         {           \"type\": \"Column\",           \"width\": \"stretch\",           \"items\": [             {               \"type\": \"TextBlock\",               \"text\": \"Matt Hidinger\",               \"weight\": \"bolder\",               \"wrap\": true             },             {               \"type\": \"TextBlock\",               \"spacing\": \"none\",               \"text\": \"Created {{DATE(2017-02-14T06:08:39Z, SHORT)}}\",               \"isSubtle\": true,               \"wrap\": true             }           ]         }       ]     },     {       \"type\": \"TextBlock\",       \"text\": \"Now that we have defined the main rules and features of the format, we need to produce a schema and publish it to GitHub. The schema will be the starting point of our reference documentation.\",       \"wrap\": true     },     {       \"type\": \"FactSet\",       \"facts\": [         {           \"title\": \"Board:\",           \"value\": \"Adaptive Card\"         },         {           \"title\": \"List:\",           \"value\": \"Backlog\"         },         {           \"title\": \"Assigned to:\",           \"value\": \"Matt Hidinger\"         },         {           \"title\": \"Due date:\",           \"value\": \"Not set\"         }       ]     }   ],   \"actions\": [     {       \"type\": \"Action.ShowCard\",       \"title\": \"Set due date\",       \"card\": {         \"type\": \"AdaptiveCard\",         \"body\": [           {             \"type\": \"Input.Date\",             \"id\": \"dueDate\"           }         ],         \"actions\": [           {             \"type\": \"Action.Submit\",             \"title\": \"OK\"           }         ]       }     },     {       \"type\": \"Action.ShowCard\",       \"title\": \"Comment\",       \"card\": {         \"type\": \"AdaptiveCard\",         \"body\": [           {             \"type\": \"Input.Text\",             \"id\": \"comment\",             \"isMultiline\": true,             \"placeholder\": \"Enter your comment\"           }         ],         \"actions\": [           {             \"type\": \"Action.Submit\",             \"title\": \"OK\"           }         ]       }     }   ] }";

for (var i = 0; i < 50; ++i)
{
    await Task.Delay(1);
    this.ContentControl.Content = null;

    var card = AdaptiveCard.FromJsonString(this.card);

    if (card != null)
    {
        var renderedAdaptiveCard = renderer.RenderAdaptiveCard(card.AdaptiveCard);

        if (renderedAdaptiveCard != null)
        {
            this.ContentControl.Content = renderedAdaptiveCard.FrameworkElement;
        }
    }
    else
    {
        await (new MessageDialog("Card was null", ":(")).ShowAsync();
    }

    await Task.Delay(100);
    GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced);
}
@shalinijoshi19
Copy link
Member

@RebeccaAnne, could you take a stab at this see if this is something that @paulcam206 has already addressed or a different one at this point (see offline email thread on this as well) ? Thanks !

@RebeccaAnne
Copy link
Contributor

Started investigation, found a few things.

  1. This is not [UWP] Memory leak in IsBackgroundImageValid #3372 recently fixed by @paulcam206. I am able to repro on the latest master which already includes that fix.

  2. This does not appear to be a regression of the issue in [UWP] Fix leaking cards #2334. Gilles added a test for that case to the test app, which seems to be in order, and is not reporting a recurrence of that leak. Also, that leak repro'd with all cards, whereas this one only repros with some cards. Which brings us to...

  3. The below seems to be a min card that repros the leak. Removing the "style": "Person" property causes the leak not to repro.

{
  "type": "AdaptiveCard",
  "version": "1.0",
  "body": [
    {
      "type": "Image",
      "style": "Person",
      "url": "https://pbs.twimg.com/profile_images/3647943215/d7f12830b3c17a5a9e4afcc370e3a37e_400x400.jpeg"
    }
  ]
}

@RebeccaAnne
Copy link
Contributor

RebeccaAnne commented Sep 13, 2019

I am able to repro a leak with the above card against the 1.0.0 nuget package.

@shalinijoshi19
Copy link
Member

Thanks @RebeccaAnne. @andycb it appears that the leak is something that has existed since at least 2017 if not longer. We'll be looking into investigating the cause and mitigating in a subsequent patch (likely early next month if not sooner) though it likely will not make the Sept patch which is supposed to be code complete in the next couple of days. We'll keep you posted. Thanks

@paulcam206
Copy link
Member

Not sure if it's the leak, but I'm addressing a couple leaks


HSTRING backgroundColor;

in #3457

@paulcam206
Copy link
Member

and quite a few more HSTRING -> HStrings in 7ce0aff

@ghost
Copy link

ghost commented Nov 29, 2019

This issue has been automatically marked as stale because it has not had any activity for 5 days.

@barryptak
Copy link
Contributor

Hi. Is there any planned work to look further in to the remaining leaks?
Thanks.

@chbomtempo
Copy link

@barryptak Maybe @shalinijoshi19 have an update on this one...

@shalinijoshi19
Copy link
Member

@paulcam206 could you double check here see if you can still repro it release/1.2 bits (or the latest released).Thanks

@RebeccaAnne RebeccaAnne added this to the Backlog milestone Jul 27, 2020
@RebeccaAnne RebeccaAnne removed this from the Backlog milestone May 6, 2021
@paulcam206
Copy link
Member

Addressing this issue is not currently on our roadmap, so closing. Please feel free to continue the conversation in this issue for future consideration.

@barryptak
Copy link
Contributor

Hey, @paulcam206.
Are you able to be clear on why this P1 bug is this not on the roadmap?
Is it because it's been mitigated significantly and so is not considered a major issue anymore?
Is it because UWP is not being supported going forward?
Is it something else?

Thanks!

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

No branches or pull requests

7 participants