cdmdotnet / Manatee.Trello

A fully object-oriented .Net wrapper for Trello's RESTful API written in C#.
MIT License
121 stars 43 forks source link

Using CardFilter.All causes huge hit in performance #299

Open gregsdennis opened 5 years ago

gregsdennis commented 5 years ago

Reported by @techfooninja

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.

techfooninja commented 5 years ago

No, I don't need actions, and disabling them did return it back to a reasonable performance for my purposes. Thanks for the workaround!

gregsdennis commented 5 years ago

Be aware that comments are actions, too. But if you need them I'd suggest refreshing them separately. Trello's API gets iffy when requesting collections within collections.

techfooninja commented 5 years ago

Thanks for the clarification. For this project, I don't need to access the comments, I just want to make sure they're not being deleted. A brief test shows that comments remain unchanged, which is perfect for my case here.

gregsdennis commented 5 years ago

This might also be related to #306.

@techfooninja, can you update to 3.10.1 and see if you have any improvements? It looks like you're not using consistency processing, so it should be faster for you.