Req #55556 [Bgs]: Better performance/optimization.

From: Date: Thu, 01 Sep 2011 07:08:22 +0000
Subject: Req #55556 [Bgs]: Better performance/optimization.
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-163213@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=55556&edit=1

 ID:                 55556
 User updated by:    Yousha_YPY at YAHOO dot co dot uk
 Reported by:        Yousha_YPY at YAHOO dot co dot uk
 Summary:            Better performance/optimization.
 Status:             Bogus
 Type:               Feature/Change Request
 Package:            *General Issues
 Operating System:   Windows/Linux...
 PHP Version:        5.3.8
 Block user comment: N
 Private report:     N

 New Comment:

Tnx for rpl.


Previous Comments:
------------------------------------------------------------------------
[2011-09-01 00:20:11] johannes@php.net

None of these changes (after converting it to C code) will bring any notable improvement.

Let's go through it:

WinUtil.c:
Yes your change might be one or two CPU cycles faster in case no path but a length was given. But
that case should never happen but indicates a bug of some kind. Marking this function as static is
even wrong as it is used from other locations which would be prevented. Using an unsigned vs. signed
int doesn't change the memory being used, it might cause some additional shifting though when
not done everywhere.

Inet.h:
C has no === operator. So I assume you are using == there. Any compiler with just a basic optimizer
should generate the same code while the current version is by many considered more readable as
having both an assignment and an comparison inside the if condition is hard to read for a human.

Sendmail.c:
Again your code is not valid C code. There's no === operator and no symbol false in C. Taking
your intention you're flipping around the order. In some cases this might safe one conditional
jump, while in the case where the jump operation is safed an addition strchr call is required (as ||
changes to &&). Depending on your test data, depending on how often mail addresses are
enclosed in <> this might make a tiny benefit while that benefit is neglectable over the time
it needs to actually send the email.

Summary:
Even if there would be an effect by these, which I can't image, it wouldn't be noteable in
an application scenario and it's way more relevant to improve overall execution performance
instead of risking bugs by changing this proven code.

------------------------------------------------------------------------
[2011-08-31 20:26:22] Yousha_YPY at YAHOO dot co dot uk

Description:
------------
Hello,
Idea's to change PHP(Extensions) for better performance/optimization:
PHP Version 5:

[WinUtil.c - Line #37]
[Original]
int php_win32_check_trailing_space(const char * path, const int path_len) {
	if (path_len < 1) {
		return 1;
	}
	if (path) {
		if (path[0] == ' ' || path[path_len - 1] == ' ') {
			return 0;
		} else {
			return 1;
		}
	} else {
		return 0;
	}
}

[Optimized]
static unsigned int php_win32_check_trailing_space(const char * path, const int path_len)
{
		if(!path)
		{
			return 0;
		}

		if((path_len < 1) || (path[0] != ' ') || (path[path_len - 1] != ' '))
		{
			return 1;
		}

	return 0;
}


[Inet.c/Inet.h - Line #79]
[Original]
int inet_aton(const char *cp, struct in_addr *inp) {
  inp->s_addr = inet_addr(cp);

  if (inp->s_addr == INADDR_NONE) {
          return 0;
  }

  return 1;
}
PHPAPI int inet_aton(const char *cp, struct in_addr *inp);

[Optimized]
unsigned int inet_aton(const char *cp, struct in_addr *inp)
{

	  if((inp->s_addr= inet_addr(cp)) === INADDR_NONE)
	  {
			return 0;
	  }

	return 1;
}
PHPAPI unsigned int inet_aton(const char *cp, struct in_addr *inp);


[SendMail.c - Line #978]
[Original]
static int FormatEmailAddress(char* Buf, char* EmailAddress, char* FormatString) {
	char *tmpAddress1, *tmpAddress2;
	int result;

	if( (tmpAddress1 = strchr(EmailAddress, '<')) && (tmpAddress2 =
strchr(tmpAddress1, '>'))  ) {
		*tmpAddress2 = 0; // terminate the string temporarily.
		result = snprintf(Buf, MAIL_BUFFER_SIZE, FormatString , tmpAddress1+1);
		*tmpAddress2 = '>'; // put it back the way it was.
		return result;
	} 
	return snprintf(Buf, MAIL_BUFFER_SIZE , FormatString , EmailAddress );
} /* end FormatEmailAddress() */
[Optimized]
static int FormatEmailAddress(char* Buf, char* EmailAddress, char* FormatString)
{
	char *tmpAddress1, *tmpAddress2;
	static unsigned int result;

		if((tmpAddress1= strchr(EmailAddress, '<')) === false && (tmpAddress2=
strchr(tmpAddress1, '>')) === false)
		{
			return snprintf(Buf, MAIL_BUFFER_SIZE , FormatString , EmailAddress );
		}

	*tmpAddress2= 0; // terminate the string temporarily.
	result= snprintf(Buf, MAIL_BUFFER_SIZE, FormatString , tmpAddress1+1);
	*tmpAddress2= '>'; // put it back the way it was.
	return result;
} /* end FormatEmailAddress() */

And...
Thanks, regards.

Test script:
---------------
None.

Expected result:
----------------
None.

Actual result:
--------------
None.


------------------------------------------------------------------------



-- 
Edit this bug report at https://bugs.php.net/bug.php?id=55556&edit=1


Thread (3 messages)

« previous php.bugs (#163213) next »