Skip to content

[WIP] Optimize pack() #18524

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion ext/standard/basic_functions.c
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,6 @@ PHP_MINIT_FUNCTION(basic) /* {{{ */

BASIC_MINIT_SUBMODULE(var)
BASIC_MINIT_SUBMODULE(file)
BASIC_MINIT_SUBMODULE(pack)
BASIC_MINIT_SUBMODULE(browscap)
BASIC_MINIT_SUBMODULE(standard_filters)
BASIC_MINIT_SUBMODULE(user_filters)
Expand Down
214 changes: 39 additions & 175 deletions ext/standard/pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
#else
#include <sys/param.h>
#endif
#include "pack.h"
#ifdef HAVE_PWD_H
#ifdef PHP_WIN32
#include "win32/pwd.h"
Expand All @@ -50,10 +49,23 @@
} \
outputpos += (a)*(b);

typedef enum {
PHP_LITTLE_ENDIAN,
PHP_BIG_ENDIAN,
} php_pack_endianness;

#ifdef WORDS_BIGENDIAN
#define MACHINE_LITTLE_ENDIAN 0
# define MACHINE_LITTLE_ENDIAN 0
# define PHP_MACHINE_ENDIAN PHP_BIG_ENDIAN
#else
#define MACHINE_LITTLE_ENDIAN 1
# define MACHINE_LITTLE_ENDIAN 1
# define PHP_MACHINE_ENDIAN PHP_LITTLE_ENDIAN
#endif

#ifdef ZEND_ENABLE_ZVAL_LONG64
# define PHP_LONG_BSWAP(u) ZEND_BYTES_SWAP64(u)
#else
# define PHP_LONG_BSWAP(u) ZEND_BYTES_SWAP32(u)
#endif

typedef ZEND_SET_ALIGNED(1, uint16_t unaligned_uint16_t);
Expand All @@ -62,40 +74,19 @@ typedef ZEND_SET_ALIGNED(1, uint64_t unaligned_uint64_t);
typedef ZEND_SET_ALIGNED(1, unsigned int unaligned_uint);
typedef ZEND_SET_ALIGNED(1, int unaligned_int);

/* Mapping of byte from char (8bit) to long for machine endian */
static int byte_map[1];

/* Mappings of bytes from int (machine dependent) to int for machine endian */
static int int_map[sizeof(int)];

/* Mappings of bytes from shorts (16bit) for all endian environments */
static int machine_endian_short_map[2];
static int big_endian_short_map[2];
static int little_endian_short_map[2];

/* Mappings of bytes from longs (32bit) for all endian environments */
static int machine_endian_long_map[4];
static int big_endian_long_map[4];
static int little_endian_long_map[4];

#if SIZEOF_ZEND_LONG > 4
/* Mappings of bytes from quads (64bit) for all endian environments */
static int machine_endian_longlong_map[8];
static int big_endian_longlong_map[8];
static int little_endian_longlong_map[8];
#endif

/* {{{ php_pack */
static void php_pack(zval *val, size_t size, int *map, char *output)
static void php_pack(const zval *val, size_t size, php_pack_endianness endianness, char *output)
{
size_t i;
char *v;
zend_ulong zl = zval_get_long(val);

convert_to_long(val);
v = (char *) &Z_LVAL_P(val);
const char *v = (const char *) &zl;
if ((endianness == PHP_LITTLE_ENDIAN) != MACHINE_LITTLE_ENDIAN) {
zl = PHP_LONG_BSWAP(zl);
v += sizeof(zend_long) - size;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this either be one of those to be extra correct with types?

Suggested change
v += sizeof(zend_long) - size;
v += sizeof(zl) - size;
Suggested change
v += sizeof(zend_long) - size;
v += sizeof(zend_ulong) - size;

}

for (i = 0; i < size; i++) {
*output++ = v[map[i]];
while (size-- > 0) {
*output++ = *v++;
}
}
/* }}} */
Expand All @@ -108,10 +99,7 @@ ZEND_ATTRIBUTE_CONST static inline uint16_t php_pack_reverse_int16(uint16_t arg)
/* {{{ php_pack_reverse_int32 */
ZEND_ATTRIBUTE_CONST static inline uint32_t php_pack_reverse_int32(uint32_t arg)
{
uint32_t result;
result = ((arg & 0xFF) << 24) | ((arg & 0xFF00) << 8) | ((arg >> 8) & 0xFF00) | ((arg >> 24) & 0xFF);

return result;
return ZEND_BYTES_SWAP32(arg);
}
/* }}} */

Expand Down Expand Up @@ -529,7 +517,7 @@ PHP_FUNCTION(pack)
case 'c':
case 'C':
while (arg-- > 0) {
php_pack(&argv[currentarg++], 1, byte_map, &ZSTR_VAL(output)[outputpos]);
php_pack(&argv[currentarg++], 1, PHP_MACHINE_ENDIAN, &ZSTR_VAL(output)[outputpos]);
outputpos++;
}
break;
Expand All @@ -538,16 +526,16 @@ PHP_FUNCTION(pack)
case 'S':
case 'n':
case 'v': {
int *map = machine_endian_short_map;
php_pack_endianness endianness = PHP_MACHINE_ENDIAN;

if (code == 'n') {
map = big_endian_short_map;
endianness = PHP_BIG_ENDIAN;
} else if (code == 'v') {
map = little_endian_short_map;
endianness = PHP_LITTLE_ENDIAN;
}

while (arg-- > 0) {
php_pack(&argv[currentarg++], 2, map, &ZSTR_VAL(output)[outputpos]);
php_pack(&argv[currentarg++], 2, endianness, &ZSTR_VAL(output)[outputpos]);
outputpos += 2;
}
break;
Expand All @@ -556,7 +544,7 @@ PHP_FUNCTION(pack)
case 'i':
case 'I':
while (arg-- > 0) {
php_pack(&argv[currentarg++], sizeof(int), int_map, &ZSTR_VAL(output)[outputpos]);
php_pack(&argv[currentarg++], sizeof(int), PHP_MACHINE_ENDIAN, &ZSTR_VAL(output)[outputpos]);
outputpos += sizeof(int);
}
break;
Expand All @@ -565,16 +553,16 @@ PHP_FUNCTION(pack)
case 'L':
case 'N':
case 'V': {
int *map = machine_endian_long_map;
php_pack_endianness endianness = PHP_MACHINE_ENDIAN;

if (code == 'N') {
map = big_endian_long_map;
endianness = PHP_BIG_ENDIAN;
} else if (code == 'V') {
map = little_endian_long_map;
endianness = PHP_LITTLE_ENDIAN;
}

while (arg-- > 0) {
php_pack(&argv[currentarg++], 4, map, &ZSTR_VAL(output)[outputpos]);
php_pack(&argv[currentarg++], 4, endianness, &ZSTR_VAL(output)[outputpos]);
outputpos += 4;
}
break;
Expand All @@ -585,16 +573,16 @@ PHP_FUNCTION(pack)
case 'Q':
case 'J':
case 'P': {
int *map = machine_endian_longlong_map;
php_pack_endianness endianness = PHP_MACHINE_ENDIAN;

if (code == 'J') {
map = big_endian_longlong_map;
endianness = PHP_BIG_ENDIAN;
} else if (code == 'P') {
map = little_endian_longlong_map;
endianness = PHP_LITTLE_ENDIAN;
}

while (arg-- > 0) {
php_pack(&argv[currentarg++], 8, map, &ZSTR_VAL(output)[outputpos]);
php_pack(&argv[currentarg++], 8, endianness, &ZSTR_VAL(output)[outputpos]);
outputpos += 8;
}
break;
Expand Down Expand Up @@ -1199,127 +1187,3 @@ PHP_FUNCTION(unpack)
}
}
/* }}} */

/* {{{ PHP_MINIT_FUNCTION */
PHP_MINIT_FUNCTION(pack)
{
int i;

if (MACHINE_LITTLE_ENDIAN) {
/* Where to get lo to hi bytes from */
byte_map[0] = 0;

for (i = 0; i < (int)sizeof(int); i++) {
int_map[i] = i;
}

machine_endian_short_map[0] = 0;
machine_endian_short_map[1] = 1;
big_endian_short_map[0] = 1;
big_endian_short_map[1] = 0;
little_endian_short_map[0] = 0;
little_endian_short_map[1] = 1;

machine_endian_long_map[0] = 0;
machine_endian_long_map[1] = 1;
machine_endian_long_map[2] = 2;
machine_endian_long_map[3] = 3;
big_endian_long_map[0] = 3;
big_endian_long_map[1] = 2;
big_endian_long_map[2] = 1;
big_endian_long_map[3] = 0;
little_endian_long_map[0] = 0;
little_endian_long_map[1] = 1;
little_endian_long_map[2] = 2;
little_endian_long_map[3] = 3;

#if SIZEOF_ZEND_LONG > 4
machine_endian_longlong_map[0] = 0;
machine_endian_longlong_map[1] = 1;
machine_endian_longlong_map[2] = 2;
machine_endian_longlong_map[3] = 3;
machine_endian_longlong_map[4] = 4;
machine_endian_longlong_map[5] = 5;
machine_endian_longlong_map[6] = 6;
machine_endian_longlong_map[7] = 7;
big_endian_longlong_map[0] = 7;
big_endian_longlong_map[1] = 6;
big_endian_longlong_map[2] = 5;
big_endian_longlong_map[3] = 4;
big_endian_longlong_map[4] = 3;
big_endian_longlong_map[5] = 2;
big_endian_longlong_map[6] = 1;
big_endian_longlong_map[7] = 0;
little_endian_longlong_map[0] = 0;
little_endian_longlong_map[1] = 1;
little_endian_longlong_map[2] = 2;
little_endian_longlong_map[3] = 3;
little_endian_longlong_map[4] = 4;
little_endian_longlong_map[5] = 5;
little_endian_longlong_map[6] = 6;
little_endian_longlong_map[7] = 7;
#endif
}
else {
zval val;
int size = sizeof(Z_LVAL(val));
Z_LVAL(val)=0; /*silence a warning*/

/* Where to get hi to lo bytes from */
byte_map[0] = size - 1;

for (i = 0; i < (int)sizeof(int); i++) {
int_map[i] = size - (sizeof(int) - i);
}

machine_endian_short_map[0] = size - 2;
machine_endian_short_map[1] = size - 1;
big_endian_short_map[0] = size - 2;
big_endian_short_map[1] = size - 1;
little_endian_short_map[0] = size - 1;
little_endian_short_map[1] = size - 2;

machine_endian_long_map[0] = size - 4;
machine_endian_long_map[1] = size - 3;
machine_endian_long_map[2] = size - 2;
machine_endian_long_map[3] = size - 1;
big_endian_long_map[0] = size - 4;
big_endian_long_map[1] = size - 3;
big_endian_long_map[2] = size - 2;
big_endian_long_map[3] = size - 1;
little_endian_long_map[0] = size - 1;
little_endian_long_map[1] = size - 2;
little_endian_long_map[2] = size - 3;
little_endian_long_map[3] = size - 4;

#if SIZEOF_ZEND_LONG > 4
machine_endian_longlong_map[0] = size - 8;
machine_endian_longlong_map[1] = size - 7;
machine_endian_longlong_map[2] = size - 6;
machine_endian_longlong_map[3] = size - 5;
machine_endian_longlong_map[4] = size - 4;
machine_endian_longlong_map[5] = size - 3;
machine_endian_longlong_map[6] = size - 2;
machine_endian_longlong_map[7] = size - 1;
big_endian_longlong_map[0] = size - 8;
big_endian_longlong_map[1] = size - 7;
big_endian_longlong_map[2] = size - 6;
big_endian_longlong_map[3] = size - 5;
big_endian_longlong_map[4] = size - 4;
big_endian_longlong_map[5] = size - 3;
big_endian_longlong_map[6] = size - 2;
big_endian_longlong_map[7] = size - 1;
little_endian_longlong_map[0] = size - 1;
little_endian_longlong_map[1] = size - 2;
little_endian_longlong_map[2] = size - 3;
little_endian_longlong_map[3] = size - 4;
little_endian_longlong_map[4] = size - 5;
little_endian_longlong_map[5] = size - 6;
little_endian_longlong_map[6] = size - 7;
little_endian_longlong_map[7] = size - 8;
#endif
}

return SUCCESS;
}
/* }}} */
22 changes: 0 additions & 22 deletions ext/standard/pack.h

This file was deleted.

1 change: 0 additions & 1 deletion ext/standard/php_standard.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
#include "php_ext_syslog.h"
#include "php_filestat.h"
#include "php_browscap.h"
#include "pack.h"
#include "datetime.h"
#include "url.h"
#include "pageinfo.h"
Expand Down