gregsdennis / Manatee.Json

A fully object-oriented approach to JSON manipulation, validation, and serialization that focuses on modeling the JSON structure rather than mere string parsing and conversion.
MIT License
198 stars 32 forks source link

Using CardFilter.All causes huge hit in performance #202

Closed techfooninja closed 5 years ago

techfooninja commented 5 years ago

I'm primarily using the Board.Cards property, since I'm interested in all cards on the board, regardless of which list they're in. When I run the following code, it executes relatively quickly (5-10 seconds for the 15-20 boards I care about):

this.trello = new TrelloFactory().Me().Result;
foreach (var board in this.trello.Boards)
{
    // board.Cards.Filter(CardFilter.All);  // <-- This causes huge perf hit
    await board.Cards.Refresh();
}

But if I uncomment the filter line, I hit a massive performance hit, possibly even more than 100x (waiting several minutes and it just finished the first board).

Details:

internal ReadOnlyActionCollection(Type type, Func<string> getOwnerId, TrelloAuthorization auth)
    : base(getOwnerId, auth)
{
    _updateRequestType = RequestTypes[type];

    EventAggregator.Subscribe(this);
}

Maybe there is a concurrency performance issue with the lock used in EventAggregator.cs on line 58?:

public static void Subscribe(IHandle subscriber)
{
    if (subscriber == null) throw new ArgumentNullException(nameof(subscriber));

    lock (Handlers)
    {
        if (Handlers.Any(x => x.Matches(subscriber))) return;

        Handlers.Add(new Handler(subscriber));
    }
}
gregsdennis commented 5 years ago

Thanks for the report. I'll check it out.

The event aggregator is actually pulled out of Caliburn.Micro and unchanged (to my recollection). I might be able to remove that lock, but I need to contemplate it first.

gregsdennis commented 5 years ago

Do you need the actions on the cards? Turning off the actions download (via Card.DownloadedFields) might be a workaround for now.

gregsdennis commented 5 years ago

Also, I think you meant to put this in Manatee.Trello, not Manatee.Json. I'm going to close this and open one over there (I'll tag you).

techfooninja commented 5 years ago

Ha, whoops...yes, I meant Manatee.Trello...sorry about that.