*: use lite runtime

[BusyJay]

We have never used any features that requires reflection, so use a lite runtime. On my machine, lite runtime compiles faster, and release compile times of TiKV changes from 15 minutes to 13 minutes.

I will check whether TiDB/PD/TiSpark requires full runtime later.

[BusyJay]

According to golang/protobuf#788, optimize_for is ignored in golang, so it should not be a problem.

[siddontang]

I am surprised that this can reduce compiling time, cool job.

LGTM

[ngaut]

Cool, LGTM

[ilovesoup]

It seems this optimization breaks Java code.

[libprotobuf WARNING ../../src/google/protobuf/compiler/java/java_file.cc:228] The optimize_for = LITE_RUNTIME option is no longer supported by protobuf Java code generator and may generate broken code. It will be ignored by protoc in the future and protoc will always generate full runtime code for Java. To use Java Lite runtime, users should use the Java Lite plugin instead. See:
  https://github.com/google/protobuf/blob/master/java/lite.md

Currently TiSpark pins to a specific version of kvproto and integration tests pulls latest version of TiDB bundle making sure that TiSpark still works with latest TiDB. So this PR will not break TiSpark instantly. However, protobuf is not only used in TiSpark, but also all other places in the platform. Instead of carefully shade it with a specific LITE version, I'd rather simply use a script removing the line while building.

[ilovesoup]

@flowbehappy please take a look at TiFlash part.

[brson]

Nice find @BusyJay !

[flowbehappy]

It breaks the build process of TiFlash, which is a CPP project. We should reconsider if this PR is suitable. According to document:

Generated classes will only implement the MessageLite interface in each language, which provides only a subset of the methods of the full Message interface.

Here is the error log:

In file included from /Users/flow/workspace/github/theflash/storage/ch/contrib/kvproto/cpp/kvproto/enginepb.grpc.pb.cc:6:
In file included from /Users/flow/workspace/github/theflash/storage/ch/contrib/kvproto/cpp/kvproto/enginepb.grpc.pb.h:9:
In file included from /usr/local/include/grpcpp/impl/codegen/async_generic_service.h:22:
In file included from /usr/local/include/grpcpp/impl/codegen/async_stream.h:22:
/usr/local/include/grpcpp/impl/codegen/call.h:307:19: error: implicit instantiation of undefined template 'grpc::SerializationTraits<enginepb::CommandRequestBatch, void>'
Status result = SerializationTraits<M, void>::Serialize(
^
/usr/local/include/grpcpp/impl/codegen/sync_stream.h:505:14: note: in instantiation of function template specialization 'grpc::internal::CallOpSendMessage::SendMessageenginepb::CommandRequestBatch' requested here
if (!ops.SendMessage(msg, options).ok()) {
^
/usr/local/Cellar/llvm@6/6.0.1/include/c++/v1/memory:2285:5: note: in instantiation of member function 'grpc::ClientReaderWriter<enginepb::CommandRequestBatch, enginepb::CommandResponseBatch>::Write' requested here
delete __ptr;
^
/usr/local/Cellar/llvm@6/6.0.1/include/c++/v1/memory:2598:7: note: in instantiation of member function 'std::__1::default_delete<grpc::ClientReaderWriter<enginepb::CommandRequestBatch, enginepb::CommandResponseBatch> >::operator()' requested here
_ptr.second()(__tmp);
^
/usr/local/Cellar/llvm@6/6.0.1/include/c++/v1/memory:2552:19: note: in instantiation of member function 'std::__1::unique_ptr<grpc::ClientReaderWriter<enginepb::CommandRequestBatch, enginepb::CommandResponseBatch>, std::__1::default_delete<grpc::ClientReaderWriter<enginepb::CommandRequestBatch, enginepb::CommandResponseBatch> > >::reset' requested here
~unique_ptr() { reset(); }
^
/Users/flow/workspace/github/theflash/storage/ch/contrib/kvproto/cpp/kvproto/enginepb.grpc.pb.h:67:14: note: in instantiation of member function 'std::__1::unique_ptr<grpc::ClientReaderWriter<enginepb::CommandRequestBatch, enginepb::CommandResponseBatch>, std::__1::default_delete<grpc::ClientReaderWriter<enginepb::CommandRequestBatch, enginepb::CommandResponseBatch> > >::~unique_ptr' requested here
return std::unique_ptr< ::grpc::ClientReaderWriter< ::enginepb::CommandRequestBatch, ::enginepb::CommandResponseBatch>>(ApplyCommandBatchRaw(context));
^
/usr/local/include/grpcpp/impl/codegen/serialization_traits.h:58:7: note: template is declared here
class SerializationTraits;
^

[BusyJay]

I'm working on a workaround to get it enabled only for rust. Hopefully stepancheg/rust-protobuf#399 can be merged, otherwise I guess I have to do some scripting.

[BusyJay]

I can successfully get it compiled on my local machine, but on CI it fails. It seems cargo in CI resolves protobuf as version 2.6.0, while my local cargo resolves it as 2.0.6. Really magical. 🤔

[Hoverbear]

@BusyJay Maybe this article can help us understand how some dependency resolution is done in Rust? Sometimes it's a puzzle to me too. :(

http://aturon.github.io/2018/07/25/cargo-version-selection/

Maybe it's related to the patch in the cargo.toml?

[brson]

The CI shouldn't be doing any crate resolution at all since the lockfile is checked in! Maybe the lockfile on this branch is not updated.

To avoid stale lockfiles in the tree I suggest running CI with --locked on every cargo command. If the build passes with --locked then you know that CI has built with the exact same deps that you did locally.

cc @zhangjinpeng1987 re adding --locked to CI invocations of cargo

[BusyJay]

I think there is a bug in cargo. After removing patch configuration, dependencies are resolved as expected. Specifically, protobuf is resolved to 2.0.6 as there is a rule '~2.0' in the Cargo.toml. But adding the patch configuration and removing Cargo.lock, grpcio ->protobuf is resolved as 2.4.

/cc rust-lang/cargo#6762

The CI shouldn't be doing any crate resolution at all since the lockfile is checked in!

No, it's not. It's a library, Cargo.lock is not expected to be checked in.

[BusyJay]

@ilovesoup @flowbehappy I use custom options to enable lite runtime now, can you check it out again?

[brson]

No, it's not. It's a library, Cargo.lock is not expected to be checked in.

Indeed. I thought I was looking at a tikv pr.

[flowbehappy]

LGTM 👍

[siddontang]

@nrc

can we use lite runtime in prost?