Declaring a new variable in a loop or outside of it?

[Saham]

PrintF vs cout?

My memory might be failing me though.

Sorry, getting somewhat off-topic.
The graduate was initializing the string variable with: char *text = ""; and then trying to strcpy to it. He was a quick learner but that was the first time I'd been confronted by a programming language having abstracted away some memory management details. It took me a long time to trust the Java garbage collector but my turning point was finding out how the Java generations and survivor spaces are handled.

For those who don't know C so well: A string had to be initialized by allocating memory to the variable. Any string operations had to make sure they didn't write past the end of the allocated memory, something you could get wrong, resulting in a buffer overflow.

 

[Kosmik]

Sorry, getting somewhat off-topic.
The graduate was initializing the string variable with: char *text = ""; and then trying to strcpy to it. He was a quick learner but that was the first time I'd been confronted by a programming language having abstracted away some memory management details. It took me a long time to trust the Java garbage collector but my turning point was finding out how the Java generations and survivor spaces are handled.

Sounds like when I first tried c#. After learning and implementing pointers and memory management in c++, it felt almost dirty changing to c#. That was back in '06, I think I actually still came across the odd garbage issue and you could manually clear it with the Dispose() method but was very very rare. I recall when we were learning on the old borland dos compiler, if you declared your arrays and pointers, it would fill them with text from the manual.txt file. If your pointer handling was out, you would get some really weird stuff coming through

 

[Saham]

... talking about performance on mailer loop is a bit ridiculous if the max tops out at 300; and really who talks performance without any mention of concurrency. As for the envisaged benefits of the setup and tear downs within the loop; this not only implies an in-depth analysis of PHP's GC but also an assumption that the value assignments don't themselves result in new heap allocations.

... if this anyway really needed to scale, then immutability would be a greater win over premature optimisation i.e. it's easier for concurrency, which is after all where performance diverges.

Plus we shouldn't overlook that the quoted code is using Microsoft's Outlook Interop with a setup / tear down every loop, which is likely a bigger issue than the one in focus... but even then I doubt this Interop's internals were designed to deal with any heavy loads and/or concurrency.

+1

First make your code understandable and only start worrying about performance when things go slow and you can run a code profiler to tell you where the bottlenecks are. I can't remember how many times I optimized my code for conditions that turned out to be unrealistic. And there were just as many times that my code was slow for reasons I didn't foresee.

 

[Solarion]

Beginning to wonder if I should put the mailer in it's own thread if it runs slow. I have not started testing yet but later I will throw test 1000 invoices at it and see what happens.

Edit: I am using a .dotx template to construct the invoice. All this adds up to overheads when there are a few hundred.

 

[MrGray]

+1

First make your code understandable and only start worrying about performance when things go slow and you can run a code profiler to tell you where the bottlenecks are. I can't remember how many times I optimized my code for conditions that turned out to be unrealistic. And there were just as many times that my code was slow for reasons I didn't foresee.

Probably a good strategy unless it's for something that scales drastically in production, in which case you need realistic load testing before deployment. Lost count of the times devs release stuff that works super fast in testing but falls down when a few dozen concurrent users hit it, and let's be honest, not everyone does a full load test every time they change a snippet of code.

 

[Saham]

Probably a good strategy unless it's for something that scales drastically in production, in which case you need realistic load testing before deployment. Lost count of the times devs release stuff that works super fast in testing but falls down when a few dozen concurrent users hit it, and let's be honest, not everyone does a full load test every time they change a snippet of code.

My favourite testing device is a separate simulator/testing app. If it creates lots of threads to generate foreseen rates of requests then it helps put the whole system under load. Even better if it can check the responses for correctness. Can also be used as a demo to reassure the client.

 

[[)roi(]]

Beginning to wonder if I should put the mailer in it's own thread if it runs slow. I have not started testing yet but later I will throw test 1000 invoices at it and see what happens.

Edit: I am using a .dotx template to construct the invoice. All this adds up to overheads when there are a few hundred.

As mentioned by @saham; focus on getting your code work as a first step; leave all the optimisation to the end, when you should anyway be doing some refactoring on your codebase.

As for the performance bottlenecks; that is going to require some investigation with timing blocks and/or Xdebug; but it's very conceivable that it's not where you have been focusing. Considering what you've shared, I think the "sendmail" function can be improved by instead of passing in a single object; you could pass in an array of objects, or a slice (if using multiple threads). However before you jump on the concurrency bandwagon, you should first check if you need this, secondly and maybe more important whether the Interop is thread safe. As for the .dotx templates; that is again another potential Interop bottleneck; so performance-wise you may consider prepping that ahead of the mailing loop.

 

[[)roi(]]

Probably a good strategy unless it's for something that scales drastically in production, in which case you need realistic load testing before deployment. Lost count of the times devs release stuff that works super fast in testing but falls down when a few dozen concurrent users hit it, and let's be honest, not everyone does a full load test every time they change a snippet of code.

True; plus for this one a simulator alone won't be the best option, unless of course they include some type of network performance retarder; because as you know email servers under load aren't always very responsive.

FYI: On the Mac, we have a free Apple developer tool called: Network Link Conditioner which helps to apply configurable level of link retardation during testing, for example:

 

[Solarion]

Thanks Roi some great advice that will help. I'll focus on getting a few successful invoice deliveries [Tests] then come back to this. I am jumping the gun a bit.

 

[MagicDude4Eva]

Java guy here: Does PHP pass variables by value or reference? If it is by reference and the send-mail function is threaded it could very well mean that by the time the mailing function accesses the variable, the next loop-iteration would have overwritten it's value.

 

[[)roi(]]

Java guy here: Does PHP pass variables by value or reference? If it is by reference and the send-mail function is threaded it could very well mean that by the time the mailing function accesses the variable, the next loop-iteration would have overwritten it's value.

Default is by value, same as Java (under the covers both tend to employ copy on write); FYI for a PHP inout reference you simply prefix the parameter with an ampersand.

As for the concern; that really only present a challenge with some form of concurrency.

 

[_kabal_]

I mostly lean towards the first open, because `MailMSG` is most likely stateful

also, if you refactor each iteration into a function, you will see it most likely makes more sense

foreach ( i in dirList) {
    object invoiceNo = i.Split("\\").Last().Split(".").First.Split(" ").Last;
    MailMSG mail_message = buildMessage("Please see attached document...", emailTo, EmailType.Invoice, i);
    SendEmail(mail_message, invoiceNo);
}

vs

MailMSG mail_message = new MailMSG();

foreach ( i in dirList) {
    object invoiceNo = i.Split("\\").Last().Split(".").First.Split(" ").Last;
    mail_message = buildMessage(mail_message, "Please see attached document...", emailTo, EmailType.Invoice, i);
    SendEmail(mail_message, invoiceNo);
}

the 2nd part just reads weird to me (obviously this is a slightly contrived example)

Also consider how this would work if this was done in parallel, and/or asyncronously

 

[[)roi(]]

I mostly lean towards the first open, because `MailMSG` is most likely stateful

also, if you refactor each iteration into a function, you will see it most likely makes more sense

foreach ( i in dirList) {
    object invoiceNo = i.Split("\\").Last().Split(".").First.Split(" ").Last;
    MailMSG mail_message = buildMessage("Please see attached document...", emailTo, EmailType.Invoice, i);
    SendEmail(mail_message, invoiceNo);
}

vs

MailMSG mail_message = new MailMSG();

foreach ( i in dirList) {
    object invoiceNo = i.Split("\\").Last().Split(".").First.Split(" ").Last;
    mail_message = buildMessage(mail_message, "Please see attached document...", emailTo, EmailType.Invoice, i);
    SendEmail(mail_message, invoiceNo);
}

the 2nd part just reads weird to me (obviously this is a slightly contrived example)

Also consider how this would work if this was done in parallel, and/or asyncronously

Agreed re second part; as for the setup function; a constructor method should suffice, and if an Interop or some similar is being used for the document template initialisation; he could simply use a separate function for this.

Declaratively this could look something like this:

• FilterInvoices -> InitInvoiceObjects -> BuildInvoicesFromTemplate -> SendEmails.

Naturally I don't know all the detail steps involved, but the basic process sounds feasible ito using a declarative bind process, which naturally lends itself to concurrency.

 

[Solarion]

Agreed re second part; as for the setup function; a constructor method should suffice, and if an Interop or some similar is being used for the document template initialisation; he could simply use a separate function for this.

Declaratively this could look something like this:

• FilterInvoices -> InitInvoiceObjects -> BuildInvoicesFromTemplate -> SendEmails.

Naturally I don't know all the detail steps involved, but the basic process sounds feasible ito using a declarative bind process, which naturally lends itself to concurrency.

That is almost exactly how I have structure the process. I will go into more detail on the project this weekend. I am also adding a CSV exporter method in there at the moment. These exported csv's Accounting can use in something like Pascal or Quickbooks.

 

[Spacerat]

In Summary; there is no obvious absolute with the approach here; but one approach can certainly lead to some surprising bugs. Plus talking about performance on mailer loop is a bit ridiculous if the max tops out at 300; and really who talks performance without any mention of concurrency. As for the envisaged benefits of the setup and tear downs within the loop; this not only implies an in-depth analysis of PHP's GC but also an assumption that the value assignments don't themselves result in new heap allocations.

... if this anyway really needed to scale, then immutability would be a greater win over premature optimisation i.e. it's easier for concurrency, which is after all where performance diverges.

Plus we shouldn't overlook that the quoted code is using Microsoft's Outlook Interop with a setup / tear down every loop, which is likely a bigger issue than the one in focus... but even then I doubt this Interop's internals were designed to deal with any heavy loads and/or concurrency.

this and this and this.

and... usually interop requires some disposal that i dont see. to create the instances in the loop and not dispose will cause serious memory leaks. Nowadays, depending on the variable usage, the compiler optimises it anyway.

The new() in .NET is so optimised that it is super fast. Rarely a consideration except cases where you need to shave microsecs.

 

[Solarion]

^

I could probably do with some adequate Interop closing

http://stackoverflow.com/questions/9962157/safely-disposing-excel-interop-objects-in-c