Lot's of memory allocations caused by using reflection.

[dosper7]

Describe the bug I've run a 10min sample where I have 20 jobs (as an example), then I've profiled with dotMemory and for that period of time, I've notice a huge memory allocation pattern related with reflection:

Current behavior Large memory allocations due to the use of reflection

Expected behavior Ideally use of source generators, but as a quick solution we could use some kind of cache to make "just once call" and avoid allocation that much amount of memory.

Version information

• NCronJobVersion:2.3.2
• .NET runtime version: net8

[linkdotnet]

Well, that isn't ideal at all. I will have to reproduce this but I do have a culprit in mind. Basically we are checking on each invocation for RetryPolicyAttribute and SupportsConcurrencyAtturbute.

[dosper7]

Yeap, don't worry, I'm already doing a PR to fix that. Just added to be registered and in the docs 📝

Today I'll do the PR 💪

[linkdotnet]

Oh nice.

I am not sure whether or not we can really leverage source generators here. But I cached dictionary should be just fine.

[linkdotnet]

Thanks for jumping in @dosper7

[linkdotnet]

Just FYI: Between 2.3.2 and the current 2.4.4 some of the internal structures have changed quite heavily. That said, make sure you are going from the latest main to reduce the chance of merge conflicts.

[dosper7]

What's the rationale behind the JobExecutionContext constructor being internal and the record being sealed? With this design, it's very hard to do unit tests against an IJob implementation.

For instance, this code doesn't work:

internal sealed class SomeScheduler : IJob
{
 Task RunAsync(JobExecutionContext context, CancellationToken token) {...}
}

In a unit test

async Task SomeTest()
{

ISomeInterface serv = new Mock<ISomeService>();
var sut = new SomeScheduler(serv); 
await sut.RunAsync(???, default); //this doesn't work anymore

serv.Verify(x => x.SomeMethod(), Times.Once());
}

I would expect some of these options:

• JobExecutionContext without the sealed (so that I can create a mock context)
• An Empty static property (so that I can use it)

[linkdotnet]

Ohh the new internal ctor is from our latest feature. For sure, we shouldn't make the ctor internal. That is an oversight.

While I do understand the rationale for unsealing the class, I don't think I am leaning towards doing so. It is a very simple object that you can easily create (well, besides the current internal ctor) so mocking is almost more complicated than newing up.

[linkdotnet]

@falvarez1 I will re-add the following to the JobExeuctionContext:

public JobExecutionContext(Type jobType, object? parameter)
    => JobDefinition = new JobDefinition(jobType, parameter, null, null);

Basically to be API-stable with <2.4. We took the ability to unit-test jobs.

[linkdotnet]

I did push a fix to make the ctor as in version <2.4. We can think of changes for 3.x.

[dosper7]

I've tested with the latest version 2.4.5 and seems it's solved 👍

[linkdotnet]

Interesting - yes we noticed we "need" (very strong word here) a Task.Delay(1); in some circumstances otherwise we will reevaluate the loop very very often. It seems that this was the effect. Nevertheless, we could "cache" the attributes on top of types, as they will never change once registered.

EDIT: Forget what I said - that is already done, that is why you don't see the spikes anymore.