.NET developers have been in the transition to move from synchronous APIs to asynchronous API. That was boosted a lot by await/async keyword of C# 5.0, but we are now in a dangerous middle ground: there are as many synchronous APIs as there are async ones. The mix of them requires the ability to call async APIs from a synchronous context, and vice versa. Calling synchronous APIs from an async context is simple – you can fire up a task and let it does the work. Calling async APIs from a sync context is much more complicated. And that is where AsyncHelper comes to the play.
AsyncHelper is a common thing used to run async code in a synchronous context. It is simple helper class with two methods to run async APIs
public static TResult RunSync<TResult>(Func<Task<TResult>> func)
{
var cultureUi = CultureInfo.CurrentUICulture;
var culture = CultureInfo.CurrentCulture;
return _myTaskFactory.StartNew(() =>
{
Thread.CurrentThread.CurrentCulture = culture;
Thread.CurrentThread.CurrentUICulture = cultureUi;
return func();
}).Unwrap().GetAwaiter().GetResult();
}
public static void RunSync(Func<Task> func)
{
var cultureUi = CultureInfo.CurrentUICulture;
var culture = CultureInfo.CurrentCulture;
_myTaskFactory.StartNew(() =>
{
Thread.CurrentThread.CurrentCulture = culture;
Thread.CurrentThread.CurrentUICulture = cultureUi;
return func();
}).Unwrap().GetAwaiter().GetResult();
}
There are slight variants of it, with and without setting the CurrentCulture and CurrentUICulture, but the main part is still spawning a new Task to run the async task, then blocks and gets the result using Unwrap().GetAwaiter().GetResult();
One of the reason it was so popular was people think it was written by Microsoft so it must be safe to use, but it is actually not true: the class is introduced as an internal class by AspNetIdentity AspNetIdentity/src/Microsoft.AspNet.Identity.Core/AsyncHelper.cs at main · aspnet/AspNetIdentity (github.com) .That means Microsoft teams can use it when they think it’s the right choice to do, it’s not the default recommendation to run async tasks in a synchronous context.
Unfortunately I’ve seen a fair share of threads stuck in AsyncHelper.RunSync stacktrace, likely have fallen victims of a deadlock situation.
Async/sync is a complex topic and even experienced developers make mistake. There is no simple way to just run async code in a sync context. AsyncHelper is absolutely not. It is simple, convenient way, but does not guarantee to be correct thing in your use case. I see it as a shortcut to solve some problems but create bigger ones down the path.
Just because you can. doesn’t mean you should. That applies to AsyncHelper perfectly
Let’s take a break from the memory allocation, and do some optimization on another aspect, yet as important (if not even more important) – database.
We all know that database queries play an essential part in any serious app. It’s almost a given that if you want your app to perform well, your database queries must also perform well. And for them to perform well, you need things like proper design (normalization, references etc.), properly written queries, and proper indexes. In this post, we will explore how an index can improve query performance, and how can we do it better.
Let’s start with a fairly simple table design
CREATE TABLE [dbo].[UniqueCoupon](
[Id] [int] identity primary key clustered,
[PromotionId] [int] NOT NULL,
[Code] [nvarchar](10) NOT NULL,
[ExpiredOn] [datetime] NULL,
[Redeemed] [bit] NULL
) ON [PRIMARY]
Nothing extraordinary here, pretty common if you ask me. Now for testing purpose, let’s insert 1.000.000 rows into it
INSERT INTO dbo.[UniqueCoupon] (PromotionId, Code)
SELECT
FLOOR(RAND()*(100)+1),
SUBSTRING(CONVERT(varchar(255), NEWID()), 0, 7)
GO 1000000
We need to query data by the code, so let’s create an user defined type
CREATE TYPE CouponTable AS TABLE (
Code NVARCHAR(10));
Time to run some query against data, let’s go with this
SELECT Id, PromotionId, Code, ExpiredOn, Redeemed FROM dbo.UniqueCoupons
WHERE PromotionId = @PromotionId AND Code in (SELECT Code FROM @Data)
This is the complete query as we need some data
declare @data CouponTable
insert into @data
select top 10 code from dbo.UniqueCoupon
where promotionid = 36
SELECT Id, PromotionId, Code, ExpiredOn, Redeemed FROM dbo.UniqueCoupon
WHERE PromotionId = 36 AND Code in (SELECT Code FROM @Data)
As we learned that execution plan is not a good way to compare performance, let’s use the statistics, our trusted friends
set statistics io on
set statistics time on
And this is how it takes with our default setting (i.e. no index)
If you are somewhat experienced with SQL Server, you might guess it would not be exactly happy because of, obviously an index is needed. As we query on PromotionId, it does makes sense to add an index for it, SQL Server does give you that
If we just blindly add the index suggested by SQL Server
If we look at the index, there’s something not very optimized about it – we are query by both PromotionId and Code, so not really makes senses to have Code as included. How’s about we have the index on both PromotionId and Code?
Yet we can make it better! From 53 to 30 logical reads might not sound a lot, but if you have thousands of queries every hour, it will be fairly significant.
Prepare yourself for some pleasant surprises – when we eventually applied the change on an actual database, the change was staggering, much more than what we hoped for. The query that were run for 24h in total, every day, now takes less than 10 minutes (yes you read it right, 10 minutes).
At this point you can certainly be happy and move on. But can we do better? For the sake of curiosity ? Yes we do.
SQL Server is rather smart that it knows we are getting the other columns as well, so those will be included in the index, to avoid a key lookup. Let’s see if we can remove that and see how it performs
So it was indeed worse, a key lookup is performed for every row (SQL Server uses the index to track down the rows and read the other columns from there)
There are two way to get rid of those key lookup – includes the columns in the index itself, or, more dramatic, make the index the clustered. As we can see the data should be accessed by PromotionId and Code, it makes perfect senses.
It is a commonly belief that Identity column should be clustered index – it is unique, it is not null. However, it only makes senses if it is the most heavily accessed column. In this case, Id only serves as an Identity column, it does not need to be the clustered index (although being an unique means it will has a non clustered index for it)
ALTER TABLE [dbo].[UniqueCoupon] DROP CONSTRAINT [PK__UniqueCo__3214EC0744C2FF38] WITH ( ONLINE = OFF )
GO
ALTER TABLE [dbo].[UniqueCoupon] ADD PRIMARY KEY NONCLUSTERED
(
[Id] ASC
)
Does this bring dramatically performance change? Unlikely. My test show no improvement in statistic. However, there is one critical impact here: we significantly reduced the size of indexes in the table. (data incoming)
Moral of the story
Indexes are crucial.
You can almost always do better than the auto suggested indexes.
Hi again every body. New day – new thing to write about. today we will talk about memory allocation, and effect it has on your website performance. With .NET, memory allocations are usually overlooked because CLR handles that for you. Except in rare cases that you need to handle unmanaged resources, that you have to be conscious about releasing that said resources yourself, it’s usually fire and forget approach.
Truth is, it is more complicated than that. The more objects you created, the more memory you need, and the more time CLR needs to clean it up after you. When you might have written code that is executed blazing fast in your benchmarks, in reality, your website might still struggle to perform well in long run – and that’s because of Garbage collection. Occasional GC is not of a concern – because it’s nature of .NET CLR, but frequent GC, especially Gen 2 GC, is definitely something you should look into and should fix, because it natively affects your website performance.
The follow up question – how do you fix that.
Of course, the first step is always measuring the memory allocations of your app. Locally you can use something like Jetbrains dotMemory to profile your website, but that has a big caveat – you can’t really mimic the actual traffic to your website. Sure, it is very helpful to profile something like a scheduled job, but it is less than optimal to see how your website performs in reality. To do that, we need another tool, and I’ve found nothing better than Application Insights Profiler trace on Azure. It will sample your website periodically, taking ETL ( event trace log) files in 220 seconds (Note, depends on your .NET version, you might download a .diagsession or a .netperf.zip file from Application Insights, but they are essentially the same inside (zipped .ETL)). Those files are extremely informative, they contains whole load of information which might be overwhelming if you’re new, but take small steps, you’ll be there.
Note that once extracted ETL can be very big – in 1GB or more range often. Perfview has to go through all that event log so it’s extremely memory hungry as well, especially if you open multiple ETL files at once. My perfview kept crashing when I had a 16GB RAM machine (I had several Visual Studio instances open), and that was solved when I switched to 32GB RAM
The first step is to confirm the allocation problems with GCStats (this is one of the extreme ones, but it does happen)
Two main things to look into – Total Allocs, i.e. the total size of objects allocated, and then the time spent in Garbage collection. They are naturally closely related, but not always. Total allocation might not be high but time for GC might be – in case of large objects allocation (we will talk about it in a later post). Then for the purpose of memory allocation analysis, this is where you should look at
What you find in there, might surprise you. And that’s the purpose of this series, point out possible unexpected allocations that are easy – or fairly easy – to fix.
In this first post, we will talk about a somewhat popular feature – Injected<T>.
We all know that in Optimizely Content/Commerce, the preferred way of dependency injection is constructor injection. I.e. if your class has a dependency on a certain type, that dependency should be declared as a parameter of the constructor. That’s nice and all, but not always possible. For example you might have a static class (used for extension methods) so no constructor is available. Or in some rare cases, that you can’t added a new parameter to the constructor because it is a breaking change.
Adding Injected<T> as a hidden dependency in your class is at least working, so can you forget about it?
Not quite!
This is how the uses of Injected<T> result in allocation of Structuremap objects – yes every time you call Injected<T>.Service the whole dependency tree must be built again.
And that’s not everything, during that process, other objects need to be created as well. You can right click on a path and select “Include item”. The allocations below are for anything that were created by `module episerver.framework episerver.framework!EPiServer.ServiceLocation.Injected1[System.__Canon].get_Service() i.e. all object allocations, related to Injected<T>
You can expand further to see what Injected<T>(s) have the most allocations, and therefore, are the ones should be fixed.
How can one fix a Injected<T> then? The best fix is to make it constructor dependency, but that might not always be possible. Alternative fix is to use ServiceLocator.GetInstance, but to make that variable static if possible. That way you won’t have to call Injected<T>.Service every time you need the instance.
There are cases that you indeed need a new instance every time, then the fix might be more complicated, and you might want to check if you need the whole dependency tree, or just a data object.
Moral of the story
Performance can’t be guessed, it must be measured
Injected<T> is not your good friend. You can use it if you have no other choice, but definitely avoid it in hot paths.
Exception handling and logging is essential part of any site. Your site will eventually run into problems, and as a developer it’s your job to make sure that you have enough – or at least helpful – information to look into problems. From my years of diagnosing and root cause analysis, these are the most common problems about exception handling and logging. You (or any developers that come after you) can thank yourself later.
Empty try catch is almost always bad
In my early days of professional programming, I saw a colleague – several years senior to me, fortunately, not at Episerver – write a lot of this code, basically in each and every method:
try {
//do stuffs
}
catch {}
You surely don’t want to show a YSOD screen to your visitors, but keep this in mind – this is almost always bad. You are trying to hide errors, and even worse, trying to swallow it. This should be a red flag for any code review.
There are cases when you truly, really want to swallow an exception. It’s rare, but it’s a reality. In such case, make sure to explain why you have an empty catch in comments.
Don’t catch exception only to throw it
Does this look familiar to you, if yes, then your code base have a problem (or actually, two)
It is wasting a try catch doing nothing of value. Log in, wrap it in a different type of exception (with probably more information, not just rethrow the exception
throw ex; actually reset the stacktrace. If you simply want to rethrow the exception (after doing meaningful stuffs with it), use throw; instead. That will preserve the precious stacktrace for exception handling at higher level.
Logging only the message is a crime
Someday, you will find an entry in your log looks like this
Object reference not set to an instance of an object.
And that’s it. This is even worse than no message – you know something is wrong, but you don’t know why, or how to fix it. It’s important to always log the full stacktrace of the exception, instead of just the message, unless you have a very good reason (I can’t think of one, can you?) not to.
Verbose is better than concise
This is an extension of the above lesson. You are already logging the entire stacktrace, but is your exception message helpful? One example is this
[InvalidOperationException: There is already a content type registered with name: PdfFile]
EPiServer.DataAbstraction.Internal.DefaultContentTypeRepository.ValidateUniqueness(ContentType contentType) +222
Which other type was registered with same name? Is it a built-in (i.e. system) type, or a type from a 3rd party library, or a type that you added but forgot yourself?
In this case, a much better exception message would be:
There are two or more content types registered with name PdfFile
Assembly1.Namespace1.PdfFile
Assembly2.Namespace2.PdfFile
Exception message is not only showing an error/unwanted situation has happened, but it needs to be helpful as well. When you log a message, you should try to add as much information as you can so the ones who will be looking into the issue can make a good guess of what is wrong, and how to fix it. There is virtually no limit on how verbose you can be, so feel free to add as much information as you need (I’m not asking you to add the entire “War and Peace” novel here, of course). You can thank me later.
