Skip to content
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

Allocate upb_stringview for table value #3

Open
wants to merge 1 commit into
base: php-hhvm
Choose a base branch
from
Open
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
94 changes: 47 additions & 47 deletions php/ext/google/protobuf/hhvm/encode_decode_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,52 +39,52 @@ class EncodeDecodeTest extends TestBase
# $this->expectFields($to);
# }

public function testEncodeDecodeOneof()
{
$m = new TestMessage();

$m->setOneofInt32(1);
assert(1 === $m->getOneofInt32());
assert(0.0 === $m->getOneofFloat());
assert('' === $m->getOneofString());
assert(NULL === $m->getOneofMessage());
$data = $m->serializeToString();
$n = new TestMessage();
$n->mergeFromString($data);
assert(1 === $n->getOneofInt32());

$m->setOneofFloat(2.0);
assert(0 === $m->getOneofInt32());
assert(2.0 === $m->getOneofFloat());
assert('' === $m->getOneofString());
assert(NULL === $m->getOneofMessage());
$data = $m->serializeToString();
$n = new TestMessage();
$n->mergeFromString($data);
assert(2.0 === $n->getOneofFloat());

$m->setOneofString('abc');
assert(0 === $m->getOneofInt32());
assert(0.0 === $m->getOneofFloat());
assert('abc' === $m->getOneofString());
assert(NULL === $m->getOneofMessage());
$data = $m->serializeToString();
$n = new TestMessage();
$n->mergeFromString($data);
assert('abc' === $n->getOneofString());

$sub_m = new TestMessage_Sub();
$sub_m->setA(1);
$m->setOneofMessage($sub_m);
assert(0 === $m->getOneofInt32());
assert(0.0 === $m->getOneofFloat());
assert('' === $m->getOneofString());
assert(1 === $m->getOneofMessage()->getA());
$data = $m->serializeToString();
$n = new TestMessage();
$n->mergeFromString($data);
assert(1 === $n->getOneofMessage()->getA());
}
# public function testEncodeDecodeOneof()
# {
# $m = new TestMessage();
#
# $m->setOneofInt32(1);
# assert(1 === $m->getOneofInt32());
# assert(0.0 === $m->getOneofFloat());
# assert('' === $m->getOneofString());
# assert(NULL === $m->getOneofMessage());
# $data = $m->serializeToString();
# $n = new TestMessage();
# $n->mergeFromString($data);
# assert(1 === $n->getOneofInt32());
#
# $m->setOneofFloat(2.0);
# assert(0 === $m->getOneofInt32());
# assert(2.0 === $m->getOneofFloat());
# assert('' === $m->getOneofString());
# assert(NULL === $m->getOneofMessage());
# $data = $m->serializeToString();
# $n = new TestMessage();
# $n->mergeFromString($data);
# assert(2.0 === $n->getOneofFloat());
#
# $m->setOneofString('abc');
# assert(0 === $m->getOneofInt32());
# assert(0.0 === $m->getOneofFloat());
# assert('abc' === $m->getOneofString());
# assert(NULL === $m->getOneofMessage());
# $data = $m->serializeToString();
# $n = new TestMessage();
# $n->mergeFromString($data);
# assert('abc' === $n->getOneofString());
#
# $sub_m = new TestMessage_Sub();
# $sub_m->setA(1);
# $m->setOneofMessage($sub_m);
# assert(0 === $m->getOneofInt32());
# assert(0.0 === $m->getOneofFloat());
# assert('' === $m->getOneofString());
# assert(1 === $m->getOneofMessage()->getA());
# $data = $m->serializeToString();
# $n = new TestMessage();
# $n->mergeFromString($data);
# assert(1 === $n->getOneofMessage()->getA());
# }

public function testEncodeDecode()
{
Expand Down Expand Up @@ -114,7 +114,7 @@ public function testEncodeDecode()
$encode_end = microtime_float();
$encode_time += $encode_end - $encode_start;

# var_dump(bin2hex($data));
var_dump(bin2hex($data));
$size = strlen($data);

// Decode Message
Expand Down
72 changes: 64 additions & 8 deletions php/ext/google/protobuf/hhvm/upb.c
Original file line number Diff line number Diff line change
Expand Up @@ -4297,16 +4297,15 @@ static uint8_t upb_msg_fielddefsize(const upb_fielddef *f) {
* a char* / size_t pair, which is too big for a upb_value. To fix this
* we'll probably need to dynamically allocate a upb_msgval and store a
* pointer to that in the tables for extensions/maps. */
static upb_value upb_toval(upb_msgval val) {
/* static upb_value upb_toval(upb_msgval val) {
upb_value ret;
memset(&ret, 0, sizeof(upb_value)); /* XXX */
ret.val = val;
return ret;
}

static upb_msgval upb_msgval_fromval(upb_value val) {
static upb_msgval upb_msgval_fromval(upb_fieldtype_t type, upb_value val) {
return val.val;
}
} */

static upb_ctype_t upb_fieldtotabtype(upb_fieldtype_t type) {
switch (type) {
Expand Down Expand Up @@ -5170,6 +5169,58 @@ static upb_msgval upb_map_fromkey(upb_fieldtype_t type, const char *key,
UPB_UNREACHABLE();
}

static void upb_map_toval(upb_fieldtype_t type, upb_msgval *val,
upb_value *out_val, upb_alloc *a) {
switch (type) {
case UPB_TYPE_STRING:
case UPB_TYPE_BYTES: {
upb_stringview *str = upb_malloc(a, sizeof(upb_stringview));
*str = val->str;
memcpy(&out_val->val, &str, sizeof(str));
return;
}
#define CASE(capitaltype, membername) \
case UPB_TYPE_ ## capitaltype: \
memcpy(&out_val->val, &val->membername, upb_msgval_sizeof(type)); \
return;

CASE(BOOL, b)
CASE(INT32, i32)
CASE(INT64, i64)
CASE(UINT32, u32)
CASE(UINT64, u64)
CASE(DOUBLE, dbl)
CASE(FLOAT, flt)
CASE(ENUM, i32)
CASE(MESSAGE, msg)

#undef CASE
}
UPB_UNREACHABLE();
}

static upb_msgval upb_map_fromval(upb_fieldtype_t type, upb_value val) {
switch (type) {
case UPB_TYPE_BYTES:
case UPB_TYPE_STRING: {
upb_msgval ret;
ret.str = **(upb_stringview**)&val.val;
return ret;
}
case UPB_TYPE_BOOL:
case UPB_TYPE_INT32:
case UPB_TYPE_UINT32:
case UPB_TYPE_INT64:
case UPB_TYPE_UINT64:
case UPB_TYPE_DOUBLE:
case UPB_TYPE_ENUM:
case UPB_TYPE_FLOAT:
case UPB_TYPE_MESSAGE:
return upb_msgval_read(&val.val, 0, upb_msgval_sizeof(type));
}
UPB_UNREACHABLE();
}

size_t upb_map_sizeof(upb_fieldtype_t ktype, upb_fieldtype_t vtype) {
/* Size does not currently depend on key/value type. */
UPB_UNUSED(ktype);
Expand Down Expand Up @@ -5241,7 +5292,7 @@ bool upb_map_get(const upb_map *map, upb_msgval key, upb_msgval *val) {
upb_map_tokey(map->key_type, &key, &key_str, &key_len);
ret = upb_strtable_lookup2(&map->strtab, key_str, key_len, &tabval);
if (ret) {
memcpy(val, &tabval, sizeof(tabval));
*val = upb_map_fromval(map->val_type, tabval);
}

return ret;
Expand All @@ -5251,16 +5302,18 @@ bool upb_map_set(upb_map *map, upb_msgval key, upb_msgval val,
upb_msgval *removed) {
const char *key_str;
size_t key_len;
upb_value tabval = upb_toval(val);
upb_value tabval;
upb_value removedtabval;
upb_alloc *a = map->alloc;

upb_map_tokey(map->key_type, &key, &key_str, &key_len);
upb_map_toval(map->val_type, &val, &tabval, a);

/* TODO(haberman): add overwrite operation to minimize number of lookups. */
if (upb_strtable_lookup2(&map->strtab, key_str, key_len, NULL)) {
upb_strtable_remove3(&map->strtab, key_str, key_len, &removedtabval, a);
memcpy(&removed, &removedtabval, sizeof(removed));
/* TODO(teboring): free memory of upb_stringview. */
*removed = upb_map_fromval(map->val_type, removedtabval);
}

return upb_strtable_insert3(&map->strtab, key_str, key_len, tabval, a);
Expand All @@ -5272,6 +5325,7 @@ bool upb_map_del(upb_map *map, upb_msgval key) {
upb_alloc *a = map->alloc;

upb_map_tokey(map->key_type, &key, &key_str, &key_len);
/* TODO(teboring): free memory of upb_stringview. */
return upb_strtable_remove3(&map->strtab, key_str, key_len, NULL, a);
}

Expand All @@ -5281,6 +5335,7 @@ bool upb_map_del(upb_map *map, upb_msgval key) {
struct upb_mapiter {
upb_strtable_iter iter;
upb_fieldtype_t key_type;
upb_fieldtype_t val_type;
};

size_t upb_mapiter_sizeof() {
Expand All @@ -5290,6 +5345,7 @@ size_t upb_mapiter_sizeof() {
void upb_mapiter_begin(upb_mapiter *i, const upb_map *map) {
upb_strtable_begin(&i->iter, &map->strtab);
i->key_type = map->key_type;
i->val_type = map->val_type;
}

upb_mapiter *upb_mapiter_new(const upb_map *t, upb_alloc *a) {
Expand Down Expand Up @@ -5321,7 +5377,7 @@ upb_msgval upb_mapiter_key(const upb_mapiter *i) {
}

upb_msgval upb_mapiter_value(const upb_mapiter *i) {
return upb_msgval_fromval(upb_strtable_iter_value(&i->iter));
return upb_map_fromval(i->val_type, upb_strtable_iter_value(&i->iter));
}

void upb_mapiter_setdone(upb_mapiter *i) {
Expand Down
34 changes: 17 additions & 17 deletions php/ext/google/protobuf/hhvm/upb.h
Original file line number Diff line number Diff line change
Expand Up @@ -966,7 +966,7 @@ typedef enum {
} upb_ctype_t;

typedef struct {
upb_msgval val;
uint64_t val;
#ifndef NDEBUG
/* In debug mode we carry the value type around also so we can check accesses
* to be sure the right member is being read. */
Expand All @@ -990,13 +990,13 @@ UPB_INLINE char *upb_gstrdup(const char *s) {
return upb_strdup(s, &upb_alloc_global);
}

UPB_INLINE void _upb_value_setval(upb_value *v, upb_msgval val,
UPB_INLINE void _upb_value_setval(upb_value *v, uint64_t val,
upb_ctype_t ctype) {
v->val = val;
SET_TYPE(v->ctype, ctype);
}

UPB_INLINE upb_value _upb_value_val(upb_msgval val, upb_ctype_t ctype) {
UPB_INLINE upb_value _upb_value_val(uint64_t val, upb_ctype_t ctype) {
upb_value ret;
_upb_value_setval(&ret, val, ctype);
return ret;
Expand All @@ -1012,7 +1012,7 @@ UPB_INLINE upb_value _upb_value_val(upb_msgval val, upb_ctype_t ctype) {
* upb_value upb_value_int32(int32_t val); */
#define FUNCS(name, membername, type_t, converter, proto_type) \
UPB_INLINE void upb_value_set ## name(upb_value *val, type_t cval) { \
val->val = upb_msgval_ ## converter(cval); \
val->val = (converter)cval; \
SET_TYPE(val->ctype, proto_type); \
} \
UPB_INLINE upb_value upb_value_ ## name(type_t val) { \
Expand All @@ -1022,18 +1022,18 @@ UPB_INLINE upb_value _upb_value_val(upb_msgval val, upb_ctype_t ctype) {
} \
UPB_INLINE type_t upb_value_get ## name(upb_value val) { \
UPB_ASSERT_DEBUGVAR(val.ctype == proto_type); \
return (type_t)upb_msgval_get ## converter(val.val); \
return (type_t)(converter)val.val; \
}

FUNCS(int32, int32, int32_t, int32, UPB_CTYPE_INT32)
FUNCS(int64, int64, int64_t, int64, UPB_CTYPE_INT64)
FUNCS(uint32, uint32, uint32_t, uint32, UPB_CTYPE_UINT32)
FUNCS(uint64, uint64, uint64_t, uint64, UPB_CTYPE_UINT64)
FUNCS(bool, _bool, bool, bool, UPB_CTYPE_BOOL)
FUNCS(cstr, cstr, char*, cstr, UPB_CTYPE_CSTR)
FUNCS(ptr, ptr, void*, ptr, UPB_CTYPE_PTR)
FUNCS(constptr, constptr, const void*, constptr, UPB_CTYPE_CONSTPTR)
FUNCS(fptr, fptr, upb_func*, fptr, UPB_CTYPE_FPTR)
FUNCS(int32, int32, int32_t, int32_t, UPB_CTYPE_INT32)
FUNCS(int64, int64, int64_t, int64_t, UPB_CTYPE_INT64)
FUNCS(uint32, uint32, uint32_t, uint32_t, UPB_CTYPE_UINT32)
FUNCS(uint64, uint64, uint64_t, uint64_t, UPB_CTYPE_UINT64)
FUNCS(bool, _bool, bool, bool, UPB_CTYPE_BOOL)
FUNCS(cstr, cstr, char*, uintptr_t, UPB_CTYPE_CSTR)
FUNCS(ptr, ptr, void*, uintptr_t, UPB_CTYPE_PTR)
FUNCS(constptr, constptr, const void*, uintptr_t, UPB_CTYPE_CONSTPTR)
FUNCS(fptr, fptr, upb_func*, uintptr_t, UPB_CTYPE_FPTR)

#undef FUNCS

Expand Down Expand Up @@ -1104,7 +1104,7 @@ UPB_INLINE char *upb_tabstr(upb_tabkey key, uint32_t *len) {
* This separate definition is necessary because in C++, UINTPTR_MAX isn't
* reliably available. */
typedef struct {
upb_msgval val;
uint64_t val;
} upb_tabval;

#else
Expand Down Expand Up @@ -1135,7 +1135,7 @@ typedef union {
} staticinit;

/* The normal accessor that we use for everything at runtime. */
upb_msgval val;
uint64_t val;
} upb_tabval;

#ifdef UPB_PTR_IS_64BITS
Expand Down Expand Up @@ -1269,7 +1269,7 @@ static const upb_tabent *upb_getentry(const upb_table *t, uint32_t hash) {
}

UPB_INLINE bool upb_arrhas(upb_tabval key) {
return key.val.u64 != (uint64_t)-1;
return key.val != (uint64_t)-1;
}

/* Initialize and uninitialize a table, respectively. If memory allocation
Expand Down