Skip to content

[PHP] New version of upb, and a new hash function wyhash in third_party. - #8000

Merged
haberman merged 7 commits into
protocolbuffers:masterfrom
haberman:php-upb-update
Nov 3, 2020
Merged

[PHP] New version of upb, and a new hash function wyhash in third_party.#8000
haberman merged 7 commits into
protocolbuffers:masterfrom
haberman:php-upb-update

Conversation

@haberman

@haberman haberman commented Nov 2, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

This brings a bunch of upb performance improvements, particularly in descriptor loading. PHP should see a significant improvement in the speed of imports of generated classes.

This includes two sets of optimizations to descriptor loading:

So overall I anticipate that Ads will see a >2x speedup in the up-front cost of importing protobuf types.

@google-cla google-cla Bot added the cla: yes label Nov 2, 2020
@haberman
haberman requested a review from acozzette November 3, 2020 16:16
@haberman
haberman merged commit 1dd4834 into protocolbuffers:master Nov 3, 2020
@haberman
haberman deleted the php-upb-update branch December 6, 2020 20:27
@stouset

stouset commented Jan 21, 2021 •
edited
Loading

Copy link
Copy Markdown

Building from source fails due to not finding wyhash.

$ cat Gemfile | grep google-protobuf
gem 'google-protobuf', git: 'https://github.com/protocolbuffers/protobuf'

$ bundle
…
convert.c:44:10: fatal error: 'third_party/wyhash/wyhash.h' file not found
#include "third_party/wyhash/wyhash.h"
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 warnings and 1 error generated.
make: *** [convert.o] Error 1

I'm guessing the appropriate -I flag isn't passed to the compiler, as the third_party directory is in the toplevel of the project, and not available in the ruby subdirectory.

@stouset

stouset commented Jan 21, 2021

Copy link
Copy Markdown

Adding

find_header('third_party/wyhash/wyhash.h', '../../../..')

to extconf.rb appears to resolve this.

@bbuchalter bbuchalter mentioned this pull request Mar 12, 2021
Closed
@haberman haberman mentioned this pull request Jul 14, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@acozzette acozzette acozzette approved these changes

Assignees

No one assigned

Labels

cla: yes php

Projects

None yet

Milestone

No milestone

Development

See More

Successfully merging this pull request may close these issues.

4 participants

@haberman @stouset @acozzette @protobuf-kokoro