8
\$\begingroup\$

I have a model I have created to generate iCal and the necessary JSON for the jQuery calendar plugin Full Calendar to work. When I display the page, the JSON generation takes about 7 seconds. The query itself has been running in only a few microseconds. So I know my code is the problem.

I was trying to get the format like what Full Calendar expects thus why I wrote the code how I did. My guess is that the below is the problem. Any thoughts on where I can improve this?

#region Event
public class Event
{
    public string Title { set; get; }
    public string Description { set; get; }
    public string URL { set; get; }
    public string UID { set; get; }
    public DateTime DateTimeStamp { set; get; }
    public DateTime CreatedDateTime { set; get; }
    public DateTime Start { set; get; }
    public DateTime End { set; get; }
    public bool AllDay { set; get; }
}
#endregion

public class Calendar
{
    #region Model
    public string CalendarName { set; get; }
    public string Product { set; get; }
    public string TimeZone { set; get; }
    public List<Event> Events { private set; get; }
    #endregion

    /// <summary>
    /// Creates a calendar json string.
    /// </summary>
    /// <returns></returns>
    public string ToFullCalendarJsonString()
    {
        StringBuilder sb = new StringBuilder();

        sb.Append("[");

        int count = Events.Count;
        int i = 0;

        foreach (Event eventItem in Events)
        {
            sb.Append("{");
            sb.AppendFormat("\"title\":\"{0}\",", eventItem.Title);
            sb.AppendFormat("\"allDay\":\"{0}\",", eventItem.AllDay);

            if (eventItem.AllDay)
            {
                sb.AppendFormat("\"start\":\"{0}T00:00:00\",", eventItem.Start.ToString("yyyy-MM-dd"));
                sb.AppendFormat("\"end\":\"{0}T00:00:00\",", eventItem.End.ToString("yyyy-MM-dd"));
            }
            else
            {
                sb.AppendFormat("\"start\":\"{0}T{1}\",", eventItem.Start.ToString("yyyy-MM-dd"), eventItem.Start.ToString("HH:mm:ss"));
                sb.AppendFormat("\"end\":\"{0}T{1}\",", eventItem.End.ToString("yyyy-MM-dd"), eventItem.End.ToString("HH:mm:ss"));
            }
            sb.AppendFormat("\"url\":\"{0}\"", eventItem.URL);
            sb.Append("}");

            i++;

            if (i < count)
            {
                sb.Append(",");
            }
        }

        sb.Append("]");

        return sb.ToString();
    }
\$\endgroup\$
  • \$\begingroup\$ This is for [asp.net-mvc-2] but I can't tag it as such. \$\endgroup\$ – Mike Wills Mar 17 '11 at 19:39
  • 2
    \$\begingroup\$ How many events do you have? \$\endgroup\$ – Snowbear Mar 17 '11 at 21:08
  • \$\begingroup\$ About 80 and more as we keep going. \$\endgroup\$ – Mike Wills Mar 18 '11 at 14:05
  • \$\begingroup\$ @Mike, well, sounds very strange. I was testing in console application and I had 5 seconds for about 20,000 events. \$\endgroup\$ – Snowbear Mar 18 '11 at 14:45
  • \$\begingroup\$ The data comes from an iSeries. I hope that isn't part of the problem. \$\endgroup\$ – Mike Wills Mar 18 '11 at 19:30
11
\$\begingroup\$

I just read somewhere that AppendFormat() can be slower than simple Append() calls. Being shocked by reading this, I decided to investigate.

For 1,000,000 empty events the times required are:

  • With AppendFormat: 9297 ticks
  • Without AppendFormat: 8268 ticks

That is a considerable difference of 11%!

I'm guessing this is due to the lookup of the arguments and such. It would be nice if AppendFormat() would be recompiled to Append() calls only by default.

This is the code:

    /// <summary>
    /// Creates a calendar json string.
    /// </summary>
    /// <returns></returns>
    public string ToFullCalendarJsonStringFaster()
    {
        StringBuilder sb = new StringBuilder();

        sb.Append( "[" );

        int count = Events.Count;
        int i = 0;

        foreach ( Event eventItem in Events )
        {
            sb.Append( "{" );

            sb.Append("\"title\":\"");
            sb.Append(eventItem.Title);
            sb.Append("\",");
            sb.Append("\"allDay\":\"");
            sb.Append(eventItem.AllDay);
            sb.Append("\",");

            if ( eventItem.AllDay )
            {
                // My test never comes here, so I left it out.
            }
            else
            {
                sb.Append("\"start\":\"");
                sb.Append(eventItem.Start.ToString("yyyy-MM-dd"));
                sb.Append("T");
                sb.Append(eventItem.Start.ToString("HH:mm:ss"));
                sb.Append("\",");

                sb.Append("\"end\":\"");
                sb.Append(eventItem.End.ToString("yyyy-MM-dd"));
                sb.Append("T");
                sb.Append(eventItem.End.ToString("HH:mm:ss"));
                sb.Append("\",");
            }
            sb.Append("\"url\":\"");
            sb.Append(eventItem.URL);
            sb.Append("\"");
            sb.Append( "}" );

            i++;

            if ( i < count )
            {
                sb.Append( "," );
            }
        }

        sb.Append( "]" );

        return sb.ToString();
    }

By also applying Snowbear's earlier replies:

  • Using yyyy-MM-ddTHH:mm:ss as format strings, gives an extra speed difference of 5%
  • Preallocating StringBuilder size, gives an extra speed difference of 6%

In total, after applying all changes, the code is 22% faster. :)

Bottomline is, there probably isn't a 'magic' solution which can make it go instant with so many events, but you can improve the speed considerably. I suggest you run the processing on a BackgroundWorker.


... this is getting even more ridiculous. Changing the date formatting to the following:

//sb.Append(eventItem.Start.ToString( "yyyy-MM-ddTHH:mm:ss" ) );
sb.Append(eventItem.Start.Year);
sb.Append("-");
sb.Append(eventItem.Start.Month);
sb.Append("-");
sb.Append(eventItem.Start.Day);
sb.Append("T");
sb.Append(eventItem.Start.Hour);
sb.Append(":");
sb.Append(eventItem.Start.Minute);
sb.Append(":");
sb.Append(eventItem.Start.Second);
sb.Append("\",");

... gives another speed increase and makes it 34% faster in total. Might slow down again if you need more specific formatting.


Beware: this last update is probably erronous. I asked a question about proper usage of PLINQ.

I haven't used Parallel LINQ (PLINQ) yet. But this seemed a nice use for it. After replacing the for by:

Events.AsParallel().AsOrdered().ForAll( eventItem =>
{
    ...
} );    

I get a total speed increase of 43%, again 9% faster. :) This is on a dual core processor. PC's with more cores should perform better. I don't know how PLINQ works exactly, but I would think it could work even faster if one iteration doesn't need to wait on another. The StringBuilder and i are exposed as closures. Anyone got any better approaches than ForAll()?

\$\endgroup\$
  • \$\begingroup\$ I've also noticed with your last approach that I got some improvement by explicit conversion of each date component to string before passing it to Append. Though I hadn't time to check it or research on it, but it was surprising to me. \$\endgroup\$ – Snowbear Mar 18 '11 at 9:11
  • \$\begingroup\$ So ... how could we have forgotten PLINQ? Anybody more experience with it? A simple ForAll already results in another 9% speed increase. \$\endgroup\$ – Steven Jeuris Mar 18 '11 at 11:45
  • 1
    \$\begingroup\$ Wow... your work is impressive on this. Thanks! I'll take a look at your suggestions. \$\endgroup\$ – Mike Wills Mar 18 '11 at 13:29
  • 1
    \$\begingroup\$ You have to use Aggregate to get the behavior you want. The specific overload I linked can aggregate over ranges of the result in parallel, and then merges all the results at the end. \$\endgroup\$ – Chris Pitman Mar 18 '11 at 14:24
6
\$\begingroup\$

Couple of tricks to improve performance (i've got around +10%, not that much though):

1) Preallocate stringBuilder size. You can roughly calculate it's total size as Events.Count * charsPerEvent
2) In this line: sb.AppendFormat("\"end\":\"{0}T{1}\",", eventItem.End.ToString("yyyy-MM-dd"), eventItem.End.ToString("HH:mm:ss")); combine two parameters into 1 using following format string for date: yyyy-MM-ddTHH:mm:ss

Also I doubt you will be able to gain performance. Maybe you can workaround it somehow? For example caching the results. Also you may try instead of creating a new string simply write it to response via HTTPHandler if Asp.Net MVC still allows to do it easily. Not sure that it will help also.

\$\endgroup\$
2
\$\begingroup\$

Note this is off topic (not a codereview solution but an idea of how to solve the performance problem.)

It's hard for me to believe that this is really the bottleneck

Are you sure that this is really the bottleneck? You can test it by returning the same constant fake json string.

  string fake2000Events = @"{.......}";

  public string ToFullCalendarJsonString()
  {
     return fake2000Events
  }

If performance is much better than calculating real data then you may think of caching results.

\$\endgroup\$
  • \$\begingroup\$ It's not off topic IMHO. It's working code, which is being reviewed, in this particular case for performance. I'd even say that's something which could be added in the FAQ. \$\endgroup\$ – Steven Jeuris Mar 18 '11 at 11:51
  • \$\begingroup\$ Well, when initially creating it I had a small result set. It was showing immediately. Now that there is real data (about 80-85 in the result set) it is now taking 11 seconds. Yes, it went up from yesterday as more data has been added. \$\endgroup\$ – Mike Wills Mar 18 '11 at 13:35
  • 2
    \$\begingroup\$ @Mike Wills: only 85? You should use a profiler and see which calls are slowest. \$\endgroup\$ – Steven Jeuris Mar 18 '11 at 13:46
  • \$\begingroup\$ @Steven Can a profiler be used when getting data from the iSeries? \$\endgroup\$ – Mike Wills Mar 18 '11 at 19:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.