Skip to content

Commit

Permalink
Aggregate Metadata Files (#7155)
Browse files Browse the repository at this point in the history 
Instead of calling initOnce of dependencies, initialize metadata of dependencies in the same file.

Needs to pass aggregate_metadata option to protoc to trigger, e.g.:
--php_out=aggregate_metadata=foo#bar:generated_dir
For each input file, transitive dependencies (including itself), whose package name has the prefix of foo or bar, will be aggregated, in which their metadata string will be aggregated in the same internalAddGeneratedFile call. For other dependencies, initOnce is called as before.

This feature is EXPERIMENTAL. DO NOT USE!!!
  • Loading branch information
@TeBoring
TeBoring committed Feb 10, 2020
1 parent 56c48ae commit ac70b7c
Show file tree
Hide file tree
Showing 8 changed files with 387 additions and 145 deletions.
3 changes: 2 additions & 1 deletion php/composer.json
View file
Expand Up @@ -23,6 +23,7 @@
}
},
"scripts": {
"test": "(cd tests && rm -rf generated && mkdir -p generated && ../../src/protoc --php_out=generated -I../../src -I. proto/empty/echo.proto proto/test.proto proto/test_include.proto proto/test_no_namespace.proto proto/test_prefix.proto proto/test_php_namespace.proto proto/test_empty_php_namespace.proto proto/test_reserved_enum_lower.proto proto/test_reserved_enum_upper.proto proto/test_reserved_enum_value_lower.proto proto/test_reserved_enum_value_upper.proto proto/test_reserved_message_lower.proto proto/test_reserved_message_upper.proto proto/test_service.proto proto/test_service_namespace.proto proto/test_wrapper_type_setters.proto proto/test_descriptors.proto) && (cd ../src && ./protoc --php_out=../php/tests/generated -I../php/tests -I. ../php/tests/proto/test_import_descriptor_proto.proto) && vendor/bin/phpunit"
"test": "(cd tests && rm -rf generated && mkdir -p generated && ../../src/protoc --php_out=generated -I../../src -I. proto/empty/echo.proto proto/test.proto proto/test_include.proto proto/test_no_namespace.proto proto/test_prefix.proto proto/test_php_namespace.proto proto/test_empty_php_namespace.proto proto/test_reserved_enum_lower.proto proto/test_reserved_enum_upper.proto proto/test_reserved_enum_value_lower.proto proto/test_reserved_enum_value_upper.proto proto/test_reserved_message_lower.proto proto/test_reserved_message_upper.proto proto/test_service.proto proto/test_service_namespace.proto proto/test_wrapper_type_setters.proto proto/test_descriptors.proto) && (cd ../src && ./protoc --php_out=../php/tests/generated -I../php/tests -I. ../php/tests/proto/test_import_descriptor_proto.proto) && vendor/bin/phpunit",
"aggregate_metadata_test": "(cd tests && rm -rf generated && mkdir -p generated && ../../src/protoc --php_out=aggregate_metadata=foo#bar:generated -I../../src -I. proto/test.proto proto/test_include.proto && ../../src/protoc --php_out=generated -I../../src -I. proto/empty/echo.proto proto/test_no_namespace.proto proto/test_empty_php_namespace.proto proto/test_prefix.proto proto/test_php_namespace.proto proto/test_reserved_enum_lower.proto proto/test_reserved_enum_upper.proto proto/test_reserved_enum_value_lower.proto proto/test_reserved_enum_value_upper.proto proto/test_reserved_message_lower.proto proto/test_reserved_message_upper.proto proto/test_service.proto proto/test_service_namespace.proto proto/test_wrapper_type_setters.proto proto/test_descriptors.proto) && (cd ../src && ./protoc --php_out=aggregate_metadata=foo:../php/tests/generated -I../php/tests -I. ../php/tests/proto/test_import_descriptor_proto.proto) && vendor/bin/phpunit"
}
}
136 changes: 72 additions & 64 deletions php/ext/google/protobuf/def.c
View file
Expand Up @@ -890,74 +890,15 @@ bool depends_on_descriptor(const google_protobuf_FileDescriptorProto* file) {
return false;
}

const upb_filedef *parse_and_add_descriptor(const char *data,
PHP_PROTO_SIZE data_len,
InternalDescriptorPoolImpl *pool,
upb_arena *arena) {
size_t n;
google_protobuf_FileDescriptorSet *set;
const google_protobuf_FileDescriptorProto* const* files;
const upb_filedef* file;
upb_status status;

set = google_protobuf_FileDescriptorSet_parse(
data, data_len, arena);

if (!set) {
zend_error(E_ERROR, "Failed to parse binary descriptor\n");
return NULL;
}

files = google_protobuf_FileDescriptorSet_file(set, &n);

if (n != 1) {
zend_error(E_ERROR, "Serialized descriptors should have exactly one file");
return NULL;
}

// Check whether file has already been added.
upb_strview name = google_protobuf_FileDescriptorProto_name(files[0]);
// TODO(teboring): Needs another look up method which takes data and length.
file = upb_symtab_lookupfile2(pool->symtab, name.data, name.size);
if (file != NULL) {
return NULL;
}

// The PHP code generator currently special-cases descriptor.proto. It
// doesn't add it as a dependency even if the proto file actually does
// depend on it.
if (depends_on_descriptor(files[0]) &&
upb_symtab_lookupfile(pool->symtab, "google/protobuf/descriptor.proto") ==
NULL) {
if (!parse_and_add_descriptor((char *)descriptor_proto,
descriptor_proto_len, pool, arena)) {
return NULL;
}
}

upb_status_clear(&status);
file = upb_symtab_addfile(pool->symtab, files[0], &status);
check_upb_status(&status, "Unable to load descriptor");
return file;
}

void internal_add_generated_file(const char *data, PHP_PROTO_SIZE data_len,
InternalDescriptorPoolImpl *pool,
bool use_nested_submsg TSRMLS_DC) {
int i;
upb_arena *arena;
const upb_filedef* file;

arena = upb_arena_new();
file = parse_and_add_descriptor(data, data_len, pool, arena);
upb_arena_free(arena);
if (!file) return;

static void internal_add_single_generated_file(
const upb_filedef* file,
InternalDescriptorPoolImpl* pool,
bool use_nested_submsg TSRMLS_DC) {
size_t i;
// For each enum/message, we need its PHP class, upb descriptor and its PHP
// wrapper. These information are needed later for encoding, decoding and type
// checking. However, sometimes we just have one of them. In order to find
// them quickly, here, we store the mapping for them.

for (i = 0; i < upb_filedef_msgcount(file); i++) {
const upb_msgdef *msgdef = upb_filedef_msg(file, i);
CREATE_HASHTABLE_VALUE(desc, desc_php, Descriptor, descriptor_type);
Expand Down Expand Up @@ -999,6 +940,73 @@ void internal_add_generated_file(const char *data, PHP_PROTO_SIZE data_len,
}
}

const bool parse_and_add_descriptor(const char *data,
PHP_PROTO_SIZE data_len,
InternalDescriptorPoolImpl *pool,
upb_arena *arena,
bool use_nested_submsg TSRMLS_DC) {
size_t i, n;
google_protobuf_FileDescriptorSet *set;
const google_protobuf_FileDescriptorProto* const* files;
const upb_filedef* file;
upb_status status;

set = google_protobuf_FileDescriptorSet_parse(
data, data_len, arena);

if (!set) {
zend_error(E_ERROR, "Failed to parse binary descriptor\n");
return false;
}

files = google_protobuf_FileDescriptorSet_file(set, &n);

for (i = 0; i < n; i++) {
// Check whether file has already been added.
upb_strview name = google_protobuf_FileDescriptorProto_name(files[i]);
// TODO(teboring): Needs another look up method which takes data and length.
file = upb_symtab_lookupfile2(pool->symtab, name.data, name.size);
if (file != NULL) {
continue;
}

// The PHP code generator currently special-cases descriptor.proto. It
// doesn't add it as a dependency even if the proto file actually does
// depend on it.
if (depends_on_descriptor(files[i]) &&
upb_symtab_lookupfile(
pool->symtab, "google/protobuf/descriptor.proto") ==
NULL) {
if (!parse_and_add_descriptor((char *)descriptor_proto,
descriptor_proto_len, pool, arena,
use_nested_submsg TSRMLS_CC)) {
return false;
}
}

upb_status_clear(&status);
file = upb_symtab_addfile(pool->symtab, files[i], &status);
check_upb_status(&status, "Unable to load descriptor");

internal_add_single_generated_file(file, pool, use_nested_submsg TSRMLS_CC);
}

return true;
}

void internal_add_generated_file(const char *data, PHP_PROTO_SIZE data_len,
InternalDescriptorPoolImpl *pool,
bool use_nested_submsg TSRMLS_DC) {
int i;
upb_arena *arena;

arena = upb_arena_new();
parse_and_add_descriptor(data, data_len, pool, arena,
use_nested_submsg TSRMLS_CC);
upb_arena_free(arena);
return;
}

PHP_METHOD(InternalDescriptorPool, internalAddGeneratedFile) {
char *data = NULL;
PHP_PROTO_SIZE data_len;
Expand Down
36 changes: 22 additions & 14 deletions php/src/Google/Protobuf/Internal/DescriptorPool.php
View file
Expand Up @@ -59,22 +59,25 @@ public function internalAddGeneratedFile($data, $use_nested = false)
{
$files = new FileDescriptorSet();
$files->mergeFromString($data);
$file = FileDescriptor::buildFromProto($files->getFile()[0]);

foreach ($file->getMessageType() as $desc) {
$this->addDescriptor($desc);
}
unset($desc);
foreach($files->getFile() as $file_proto) {
$file = FileDescriptor::buildFromProto($file_proto);

foreach ($file->getEnumType() as $desc) {
$this->addEnumDescriptor($desc);
}
unset($desc);
foreach ($file->getMessageType() as $desc) {
$this->addDescriptor($desc);
}
unset($desc);

foreach ($file->getMessageType() as $desc) {
$this->crossLink($desc);
foreach ($file->getEnumType() as $desc) {
$this->addEnumDescriptor($desc);
}
unset($desc);

foreach ($file->getMessageType() as $desc) {
$this->crossLink($desc);
}
unset($desc);
}
unset($desc);
}

public function addMessage($name, $klass)
Expand Down Expand Up @@ -149,8 +152,13 @@ private function crossLink(Descriptor $desc)
switch ($field->getType()) {
case GPBType::MESSAGE:
$proto = $field->getMessageType();
$field->setMessageType(
$this->getDescriptorByProtoName($proto));
$subdesc = $this->getDescriptorByProtoName($proto);
if (is_null($subdesc)) {
trigger_error(
'proto not added: ' . $proto
. " for " . $desc->getFullName(), E_ERROR);
}
$field->setMessageType($subdesc);
break;
case GPBType::ENUM:
$proto = $field->getEnumType();
Expand Down
1 change: 1 addition & 0 deletions php/src/Google/Protobuf/Internal/Message.php
View file
Expand Up @@ -94,6 +94,7 @@ private function initWithGeneratedPool()
$this->desc = $pool->getDescriptorByClassName(get_class($this));
if (is_null($this->desc)) {
user_error(get_class($this) . " is not found in descriptor pool.");
return;
}
foreach ($this->desc->getField() as $field) {
$setter = $field->getSetter();
Expand Down
2 changes: 2 additions & 0 deletions php/tests/proto/test_import_descriptor_proto.proto
View file
@@ -1,5 +1,7 @@
syntax = "proto3";

package foo;

import "google/protobuf/descriptor.proto";

message TestImportDescriptorProto {
Expand Down
1 change: 1 addition & 0 deletions php/tests/well_known_test.php
View file
Expand Up @@ -4,6 +4,7 @@
require_once('test_util.php');

use Foo\TestMessage;
use Foo\TestImportDescriptorProto;
use Google\Protobuf\Any;
use Google\Protobuf\Api;
use Google\Protobuf\BoolValue;
Expand Down

0 comments on commit ac70b7c

Please sign in to comment.