Skip to content

test: refactor header inclusion to speed up building (for test/common/http)#12775

Merged
asraa merged 3 commits intoenvoyproxy:masterfrom
foreseeable:refactor_c_http
Aug 26, 2020
Merged

test: refactor header inclusion to speed up building (for test/common/http)#12775
asraa merged 3 commits intoenvoyproxy:masterfrom
foreseeable:refactor_c_http

Conversation

@foreseeable
Copy link
Contributor

Signed-off-by: Muge Chen mugechen@google.com

Commit Message: refactor header inclusion to speed up building
Additional Description: This is the follow up PR for dividing the monolithic mock header test/mocks/upstream/mocks.h #12048
We refactored mock class include directives, this will reduce the target size and building time.
Risk Level: low
Testing: existing tests
Docs Changes: N/A
Release Notes: no

Signed-off-by: Muge Chen <mugechen@google.com>
Signed-off-by: Muge Chen <mugechen@google.com>
Signed-off-by: Muge Chen <mugechen@google.com>
@derekargueta
Copy link
Member

Out of curiosity, do you have benchmark numbers of how much this speeds up the build?

@foreseeable
Copy link
Contributor Author

foreseeable commented Aug 24, 2020

Out of curiosity, do you have benchmark numbers of how much this speeds up the build?

@derekargueta
Here is the building time comparison for affected tests:

test_name after before
//test/common/http:async_client_impl_test 91.241s 113.855s
//test/common/http:codec_client_test 72.036s 71.402s
//test/common/http:conn_manager_impl_fuzz_test 81.805s 95.341s
//test/common/http:conn_manager_impl_test 106.559s 106.681s
//test/common/http/http1:conn_pool_test 70.243s 72.604s
//test/common/http/http2:conn_pool_test 73.546s 67.012s

For some test the building time does not change much, since upstream_mocks is not the bottleneck in compilation.

@derekargueta
Copy link
Member

Super cool, thanks for sharing!

Copy link
Contributor

@asraa asraa left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM besides one.

Copy link
Contributor

@asraa asraa left a comment

Choose a reason for hiding this comment

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

Thanks!

@asraa asraa merged commit 6b1901a into envoyproxy:master Aug 26, 2020
@rgs1
Copy link
Member

rgs1 commented Aug 26, 2020

@foreseeable was this done with a tool or manually?

@foreseeable
Copy link
Contributor Author

it's done with a tool @rgs1 #12150

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants