Cache purge tweaking

[denisborovnev]

Hi,

We encountered high memory usage by Dapper and mostly due to DynamicParameters.paramReaderCache.

We have multi-tenant application and to overcome parameters sniffing problem we add to some of our queries comment like -- TenantId = XX UserId = YY. I know that we can use RECOMPILE hint, but we intentionally use our approach to let sql server cache execution plans. Also some queries are generated dynamically based on user selection (filters, columns etc.).
All these leads to a huge amount of different queries and cache entries.

The same about SqlMapper._queryCache, but it is not as big as paramReaderCache because Dapper purges queries that have been executed only once (if I'm not mistaken about it).

Like a workaround we can replace such code

var parameters = new DynamicParameters();
parameters.AddDynamicParams(new { TenantId = tenantId, UserId = userId });

with

var parameters = new DynamicParameters();
parameters.Add("TenantId", tenantId);
parameters.Add("UserId", userId);

But we don't want to restrict our usage of library.

I would be happy try to add ability to configure cache purging policy that wouldn't make any changes to existing behavior by default but would allow to configure some limits for cache size.

But before trying to implement it I would like to ask if you are open to such changes and if you have any suggestions how to do it in the best way.

Thanks in advance!

[mgravell]

Question: why are you using DynamicParameters in this scenario? The most efficient usage is simply:

conn.ExecuteOrWhatever(sql, new { TenantId = tenantId, UserId = userId });

Is there a specific thing you're trying to do additional to that?

As for the query cache side of things: Dapper.AOT fixes that entirely, via the pragmatic approach of not needing one - all the work happens at build time. See here for an intro.

[denisborovnev]

We have quite complex logic for query building that is implemented in a few classes, i.e. Filter object can return part of WHERE clause and parameters for it.
We chose DynamicParameter to keep all the parameters for query and this allows add them dynamically. But in some places we use DynamicParameter.AddDynamicParams to add multiple parameters at once just because it is more convenient (or just create an initial store for query parameters like new DynamicParameter(new { ... })).

Maybe we use DynamicParameter in a wrong way but that's what we have now :)

Also in some places we use SqlBuilder and code like this also leads to template adding to DynamicParameter:

builder.Where(" StartDate = @StartDate", new { StartDate });

As for the query cache side of things: Dapper.AOT fixes that entirely

Due to dynamic queries building I'm not sure that AOT would work well for us, but we'll try it out.

[mgravell]

OK, that kind of dynamic scenario is not unreasonable, although note that if the potential args are rigid (just: not always used), then in most cases Dapper will figure out automatically which to send. I'd need to check the state of DynamicParameters with AOT - it isn't necessarily a problem.

[denisborovnev]

Hi @mgravell , I've added a small sample here.

I tried enable AOT and AOT reported warnings about dynamic SQL. But even when I tried use a static query I still couldn't find any generated code for interceptor, so looks I'm doing something wrong.

I'll try dig deeper in a few days

[mgravell]

I'll try to grab a moment to look at the sample; I thought we'd enabled that path...

[denisborovnev]

Hi!

Sorry for the delay, I've finally had a chance to look into why AOT isn't working in my sample:

• DAP016 - AOT doesn't support generic return types
• DAP015 - AOT doesn't support "dynamic" parameters. see DapperAnalyzer and IsDynamicParameters

From what I understand, getting AOT to work in our case is quite difficult :(

What do you think about making the cache size configurable? I totally understand if you don't want to make Dapper more complex, especially if this problem is not common.

For now, we’ve come up with a workaround: we check if the cache size increases after each test, which helps us detect all usages of DynamicParameters with anonymous types and replace them with named parameters.

[njaysoftware]

Hi!

The app I'm working on has also ended up with a large memory size around the DynamicParameters.paramReaderCache dictionary . Part of the issue was our integration with dapper. We were dynamically generating param names which bloated the paramReaderCache 🙃. We are make strives to remove that. However, it would be nice to have a cache eviction or limit for queries that never get hit again.

For our use case, I would rather, every once in a while, have a slow running query miss from the cache than OOM exceptions that bring down the app. I understand that might not be a shared opinion or others would rather have OOM exceptions if their cache gets to big.

Just throwing ideas out there, I noticed the SqlMapper._queryCache has a method to clear the cache, and a cache eviction once the cache hits a 1000 identities. Would the Dapper team be open to implementing a similar cache policy for the DynamicParameters.paramReaderCache? If the cache policy did change for the DynamicParameters.paramReaderCache would the preference be to maintain the old behavior and have the cache eviction be a configurable opt in?

I would be happy to try my hand at a pull request.