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

Prepare ORTools for static compilation in Rust #428

Draft
wants to merge 7 commits into
base: development
Choose a base branch
from

Conversation

flyingsilverfin
Copy link
Member

@flyingsilverfin flyingsilverfin commented Mar 16, 2023

What is the goal of this PR?

We modify the OR-Tools dependency to be statically compiled into a project, rather than linked dynamically. This leverages the fact that OR-Tools is a Google repository that exposes Bazel targets to build with. As a result, we follow the model we use for RocksDB, which is to statically compile a platform-native library into our project on demand.

Note that the OR-Tools libraries require C++20 to compile, which we set in a platform-specific way in the bazel configuration file (.bazelrc)

What are the changes implemented in this PR?

  • Remove rules related to using dynmically linked OR-Tools libraries
  • Include ORTools's github repository via Bazel, plus all of its transitive dependencies
  • Point the existing CXX bridge to generate the rust library based on the C++ sources instead of C++ header + pre-compiled dynamic libraries

Justification

Like RocksDB, ORTools was initially considered as a dynamically linked library. However, we have discovered that the Rust+Bazel ecosystem does not support packaging dynamic libraries and binaries together elegantly, and is instead geared toward static compilation.

For RocksDB, we rely on the rocksdb-sys crate to perform the compilation of RocksDB's C++ core into a Rust library. There is no such equivalent crate for ORTools, the existing crates are wrappers around dynamically linked ORTools distributions.

The solution for now is to leverage our existing work to build our own CXX bridge between Rust and C++. Instead of building this bridge on top of header files and dynamic libraries that come pre-compiled, we utilise ORTools' Bazel targets to statically compile the library for the current platform.

Note: as a consequence of this, projects using OR-tools from this repository will be much slower: compiling OR-tools can take some time, on top of the time required to clone its Github repository (~200mb). However, the compile time can be cut drastically by using a bazel build cache.

@typedb-bot
Copy link
Member

PR Review Checklist

Do not edit the content of this comment. The PR reviewer should simply update this comment by ticking each review item below, as they get completed.


Trivial Change

  • This change is trivial and does not require a code or architecture review.

Code

  • Packages, classes, and methods have a single domain of responsibility.
  • Packages, classes, and methods are grouped into cohesive and consistent domain model.
  • The code is canonical and the minimum required to achieve the goal.
  • Modules, libraries, and APIs are easy to use, robust (foolproof and not errorprone), and tested.
  • Logic and naming has clear narrative that communicates the accurate intent and responsibility of each module (e.g. method, class, etc.).
  • The code is algorithmically efficient and scalable for the whole application.

Architecture

  • Any required refactoring is completed, and the architecture does not introduce technical debt incidentally.
  • Any required build and release automations are updated and/or implemented.
  • Any new components follows a consistent style with respect to the pre-existing codebase.
  • The architecture intuitively reflects the application domain, and is easy to understand.
  • The architecture has a well-defined hierarchy of encapsulated components.
  • The architecture is extensible and scalable.

@@ -1,3 +1,9 @@
common --enable_platform_specific_config
Copy link
Member Author

Choose a reason for hiding this comment

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

We set different build flags per-host.
This is because OR-tools compilation requires C++20, and the flag to set this is different for windows MSVC compilers and non-MSVC compilers.

Note that this flag is another barrier to cross-compilation, if that ever comes back onto the table.

Copy link
Member Author

Choose a reason for hiding this comment

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

This command basically is a note to bazel that it should should select platform-specific configurations according to the host platform

@flyingsilverfin flyingsilverfin marked this pull request as ready for review March 16, 2023 10:57
load("//library/ortools/cc:deps.bzl", "google_or_tools")
google_or_tools()
load("@com_google_protobuf//:protobuf_deps.bzl", google_or_tools_protobuf_deps = "protobuf_deps")
google_or_tools_protobuf_deps()
Copy link
Member Author

Choose a reason for hiding this comment

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

or tools has its own protobuf dependencies

visibility = ["//visibility:public"]
)
""".format(lib)
def google_or_tools():
Copy link
Member Author

Choose a reason for hiding this comment

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

or_tools and all of its dependencies. We get this from the template OR-tools bazel project: https://github.com/or-tools/bazel_or-tools

git_repository(
name = "com_google_ortools",
remote = "https://github.com/google/or-tools.git",
tag = "v9.3",
Copy link
Member Author

Choose a reason for hiding this comment

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

we use a tag instead of commit + shallow_since to reduce the size of the initial git clone

Comment on lines +23 to +24
std::unique_ptr<MPSolver> new_mpsolver_sat() {
return std::unique_ptr<MPSolver>(MPSolver::CreateSolver("SAT"));
Copy link
Member Author

Choose a reason for hiding this comment

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

Expose the SAT interface explicitly

@flyingsilverfin
Copy link
Member Author

Moving back to draft until I can come back to this to finish it up

@flyingsilverfin flyingsilverfin marked this pull request as draft April 6, 2023 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants