Reputation: 4308
I am making a program for a university that grabs active students and then spits out a report for other processes to use.
One of the important functions is to see if a student has graduated or not.
When a student passes through the main function, it takes about 5 seconds to run it through the process. I found that the most time taking part of the process comes from an IQueryable.First()
in this function.
public static bool ContinuingEducation(string v)
{
var TERMSSTU = from t in _DB.TERMs
join stu in _DB.STUDENT_TERMS_VIEWs
on t.TERMS_ID equals stu.STTR_TERM
where v == stu.STTR_STUDENT
orderby t.TERM_START_DATE descending
select new { startdate = t.TERM_START_DATE };
var graduation = from a in _DB.ACAD_CREDENTIALs
where v == a.ACAD_PERSON_ID
orderby a.ACAD_COMMENCEMENT_DATE ascending
select a;
if (graduation.Count() > 0 && TERMSSTU.Count() > 0)
{
if (TERMSSTU.First().startdate > graduation.First().ACAD_COMMENCEMENT_DATE) // the problem is here
return true;
}
return false;
}
I do not know why it takes so long here. Is there a better way to write out this function so it is faster?
Upvotes: 0
Views: 238
Reputation: 4308
My best bet to get some serious efficiency was to query those records in the beginning of my program so all the records are in memory which made querying a breeze for the program.
I made a new class that captures those two queries
public class TERMSTU { public DateTime? startdate { get; set; } public String Id { get; set; } }
public class Graduation { public DateTime? ACAD_COMMENCEMENT_DATE { get; set; } public String Id { get; set; } } public class Report { private List TERMSSTUS; private List GRADUATIONS; public Report(ColleagueDataContext _DB) { var TERMSSTU = from t in _DB.TERMs join stu in _DB.STUDENT_TERMS_VIEWs on t.TERMS_ID equals stu.STTR_TERM orderby t.TERM_START_DATE descending select new { startdate = t.TERM_START_DATE, id = stu.STTR_STUDENT }; TERMSSTUS = new List(); foreach (var i in TERMSSTU) { TERMSSTUS.Add(new TERMSTU() { Id = i.id, startdate = i.startdate }); }
var graduation = from a in _DB.ACAD_CREDENTIALs
where a.ACAD_COMMENCEMENT_DATE != null
where a.ACAD_COMMENCEMENT_DATE < DateTime.Today
orderby a.ACAD_COMMENCEMENT_DATE descending
select a;
GRADUATIONS = new List<Graduation>();
foreach (var i in graduation)
{
GRADUATIONS.Add(new Graduation() { Id = i.ACAD_PERSON_ID, ACAD_COMMENCEMENT_DATE = i.ACAD_COMMENCEMENT_DATE });
}
}
public List<TERMSTU> givestu()
{
return TERMSSTUS;
}
public List<Graduation> givegrad()
{
return GRADUATIONS;
}
}
I modified the function to take the two queries
public static bool ContinuingEducation(string v, List<TERMSTU> t, List<Graduation> g)
{
var term = (from stu in t where stu.Id == v select stu).FirstOrDefault();
var grad = (from gradu in g where gradu.Id == v select gradu).FirstOrDefault();
return term.startdate > grad.ACAD_COMMENCEMENT_DATE.Value.AddDays(-14);
}
This method has more code to write, but it saves allot of time when the program has to go through 3,000 records or so.
Upvotes: 0
Reputation: 505
Try Code
public static bool ContinuingEducation(string v)
{
var TERMSSTU = (from t in _DB.TERMs.AsQueryable()
join stu in _DB.STUDENT_TERMS_VIEWs.AsQueryable()
on t.TERMS_ID equals stu.STTR_TERM
where v == stu.STTR_STUDENT
orderby t.TERM_START_DATE descending
select new { startdate = t.TERM_START_DATE }).FirstOrDefault();
var graduation = (from a in _DB.ACAD_CREDENTIALs.AsQueryable()
where v == a.ACAD_PERSON_ID
orderby a.ACAD_COMMENCEMENT_DATE ascending
select a).FirstOrDefault();
if (graduation==null || TERMSSTU==null)
return false;
return TERMSSTU.startdate >graduation.ACAD_COMMENCEMENT_DATE;
}
Upvotes: 2
Reputation: 23984
Your use of Count
is inefficient, since you need to query the database extra times (once to get the Count
, once to get the First
). The below code change will remove the need to get the Count
.
Change:
if (graduation.Count() > 0 && TERMSSTU.Count() > 0)
{
if (TERMSSTU.First().startdate > graduation.First().ACAD_COMMENCEMENT_DATE) // the problem is here
return true;
}
return false;
to:
var grad = graduation.FirstOrDefault();
var term = TERMSSTU.FirstOrDefault()
if (grad == null || term == null)
return false;
return term.startdate > grad.ACAD_COMMENCEMENT_DATE;
Note that even after making this change, the FirstOrDefault
will still likely be slow. To fix this, you should run a SQL Trace
to see what SQL is being generated. Then look to add indexes / optimise the query.
Upvotes: 4
Reputation: 1096
The performance problem is not in First
call but in your query. It is evaluated when you call First
.
You should rewrite you code to use less queries: eq you can join both queries on the v
parameter and then get the result and check it's dates. It will result in a single query.
Upvotes: 0