RFC: Update the Gem Templates to support a Public/Private API split #37
Comments
|
I think this is a great idea. It will be a cleaner separation of concerns, and will make users more cognizant of the potential side-effects when making CMake changes to their gems. I also agree we should start by updating our templates first, and then do a sweep of updating our existing gems. This will likely need input from various gem owners to determine which things can/should be in their Public vs. Private APIs. |
|
I like the idea. Couple question:
Could we be increasing the difficulty for customers to learn how to add content to gems? |
|
Thanks for writing this RFC @lumberyard-employee-dm. Overall I'm onboard with adding the new API targets to template gems. It paves the way for Gem authors to think on how they are going to present publicly their functionality while keeping the implementation private. Feedback on the document
Question
Concerns
On top of this now we are going to add 2 more projects, making 8 targets per Gem. And this is ignoring the Benchmark target, which we should be encouraging to do as well.
This comes with the risk of potential disadvantages that the RFC does not mention. If O3DE scales in number of gems, the VS solution could become very large, slow, difficult to navigate, difficult to search and find things. I think at least the RFC should touch on this subject and mention a mitigation if there is any. Alternative There is an alternative that the RFC document doesn't mention, which is to keep using the static and module targets and educate the user to make a better usage of the visibility properties PUBLIC and PRIVATE on the Includes and Dependencies. The static target can have their includes PRIVATE, that way anybody outside the Gem that tries to depend on them won't find the headers. Then the module target will make the I still prefer the proposed solution of adding API targets, because it's more explicit. But if something goes wrong it's good that the RFC mentions this as an alternative. |
For your first question: Compilation time doesn't change at all. Onto the second question: |
Answers: Feedback on the document
Answers: Question.
To answer your question, the Gem Template is only meant has a starting place for users to create there own gems. Now what a gem is really a package with a descriptor "gem.json" file and a CMakeLists.txt script that allows hooking into the O3DE Build System as a plugin. Answers: Concerns
That is out of scope for this document. Now for runtime dependencies that is actually explained in the Settings Registry Developer documentation on how gems are loaded. That documentation for the Settings Registry is not yet brought over.
Also Visual Studio isn't the only IDE out there, that exposes CMake targets as projects(Xcode, VSCode CMake extension, CLion, etc...). For Visual Studio at least, the cost of loading a project and indexing is based on the content within the .vcxproj files. The reason the Windows Visual Studio project has so many projects is because we have 80 gems within the engine. The main mitigation for the high number of projects is to move to reduce the number of gems that come with the engine. Answers: AlternativeFor the alternative approach you mentioned, static library targets still have the issue that it duplicates the object files into a Here it can be seen that we have 11GiB of STATIC libraries just for the non-monolithic profile configuration alone, most of which are private targets only used within the engine and not meant for consumption for non-engine gems. Furthermore I think the naming scheme of |
|
Closing this out as the split has been performed |
|
I would like to add that CMake and OBJECT libraries have additional dependency problems as we've discovered. The RFC is still valid, but I'd also recommend against using any macros such as (We should not be shipping This essentially means that an Another reason not to use |
|
Using an
Comparing these two methods for the Atom_RPI.Public target (with some code changes to make it link + WINDOWS_EXPORT_ALL_SYMBOLS cmake option, Ninja generator) shows the following behavior:
Based on this data I see some potential for improving engine developer productivity when using shared libraries for these kinds of targets, as this exact behavior of needing to relink numerous targets is commonly observed in our work with the engine. |
Summary:
The O3DE Template system provides several gem templates that users instantiate in order to start iteration on a new gem.
Those Gems templates need expose several CMake targets that allows a gem to build it's code into a library that is loaded by O3DE applications as a "plugin", as well as targets for sharing a gem public API with outside dependencies(other gems and libraries) as well as private targets for sharing code among the gem "plugin" and test targets.
Currently as it stands , the gem templates exposes a
${GemName}.Staticlibrary target whose purpose is to share source files and include directories among the gem plugin targets. It is meant to be a private target not used outside of gem.Additionally the gem templates exposed a
${GemName}and${GemName}.Editortarget that are a MODULE library type in non-monolithic builds and a static library in monolithic builds.CMake enforces that MODULE library targets can't be linked into other targets can only be loaded dynamically using dlopen functionality. See CMake add_library documentation for more details.
Since monolithic builds are meant for release of a game or simulation application, all the shared libraries targets are converted to the static library targets and linked into the final application in order to prevent shared library dependency issues, reduce the size of the overall binary folder and to allow the linker to perform de-duplication and optimizations over all the code.
What is the motivation for this suggestion?
Why is this important?
Having a convention for separating a private vs public API within the gem templates allow authors of gems to enforce boundaries as to what functionality a dependents libraries should use and what functionality is internal functionality that is subject to change among versions.
Furthermore a public API convention allows users of gem to learn a recommended approach for using other how to depend on the code of other gems
What are the use cases for this suggestion?
The primary use case is for dependent gems and other libraries to be able to know which targets they can depend on as part of the public API and therefore have an expectation of gem functionality that is meant to be stable.
This also empowers authors of gem to separate their internal implementation from their public exposed API, which allows them to set user expectations of updates they make to their gems.
Gem authors would be allowed to change their internal implementation with little push back as public users wouldn't depend on that implementation. When it comes to updating the public API gem authors have to be cognizant of changes they make as to not break other gems using that public API.
This creates an unspoken contract between the gem maintainer and the gem user, that the gem maintainer is allowed to change the internal implementation in anyway they would like without external insight, while any public API changes can and should receive scrutiny from users of that gem
What should the outcome be if this suggestion is implemented?
The following Gem Templates should be updated as of 2022-04-26
CppToolGem
DefaultGem
PythonToolGem
Suggested design description:
The first update to those gem would be to add a new
${GemName}.APIand${GemName}.Editor.APIINTERFACE target that exposes the public API of the gem.The those targets are meant to transfer over the interface include directories, compile definitions and options, and build dependencies that any outside gem/library using that gem needs to depend on.
By default the following gem variant API aliases will be added to facilitate users depending on the Gem API of variants instead of the runtime and editor API targets.
Here is the list of new variant API aliases
${GemName}.Clients.API${GemName}.Servers.API${GemName}.Tools.API${GemName}.Builders.APIThe next update is to create Gem variant aliases for the API targets. This allows a gem to expose a specific API based on the Gem variants that it is used with.
For example a gem could expose a different API to the AssetProcessor than the Editor by making a separate
${GemName}.Builders.APIINTERFACE target vs${GemName}.Editor.APIINTERFACE target.Next the
${GemName}.StaticSTATIC library target should change to an OBJECT target be renamed to a private${GemName}.Private.Objectas part of the Gem template(this also applies to the${GemName}.Editor.Staticbeing renamed to${GemName}.Editor.Private.Object).Why switch the STATIC library over to a OBJECT target?
An OBJECT target in CMake allow developers to compile source files into a collection of object files without creating a static or shared library. More information on an OBJECT target can be found here.
This has the major benefit that unnecessary copies of object files aren't stored in a static library, thereby saving disk space.
Furthermore this de-clutters the SDK
libdirectory as it would only contain public static libraries that aren't meant to be used by other targets.An example where this helps is with the
LyShine.Staticlibrary target which should be private to the LyShine Gem. In a build of the source engine, there would be two copies of the object files that get build for it, one on disk and one inside of the "LyShine.Static.a" archive file, but if the target is an OBJECT target, then there would be no "LyShine.Static.a" file.Because there is no "LyShine.Static.a" file, when creating an SDK layout, there is no copying of that private library to the SDK
What are the advantages of the suggestion?
The gem maintainer, has a framework in which to expose public functionality, while reserving a section of their gem from making changes to the implementation without worrying that they are breaking external users.
This make it easier to implement a version scheme for code changes within a gem. Perhaps API breaking changes are major version bumps, while internal implementation changes are minor changes
Users of newer gems will now have a known target name they can depend on in their libraries to access the functionality of the gem.
What are the disadvantages of the suggestion?
The disadvantage is that existing gems will not have the new API targets.
The expectation for users will be that to add a dependency on a gem API, they would only need to a dependency on
${GemName}.<variant>.APItarget.User could be surprised when coming across gems that don't use the convention. This creates a bit of an inconsistency between older gems and newer gems.
Using the updated Gem Template would result in 1 to 2 more TARGETS being created out of the box by default.
If using an IDE such as Visual Studio with a Gem created from the Default Gem template, this will result in 8 .vcxproj being loaded instead of 6. This is balanced with the
.Staticvcxproj is being split into a smaller.APIand.Private.Objectvcxproj targets which loads faster.How will this be work within the O3DE project?
This should be seamless. Users would use the same
o3de.py create-gemcommand to create their gems with the new public API and private object targets.Are there any alternatives to this suggestion?
There is a feature request with CMake to allow specification of DIRECTORY scope for regular CMake Targets: https://gitlab.kitware.com/cmake/cmake/-/issues/23433
That would allow enforcement through cmake other gems couldn't depend on the private
${GemName}.Statictargets making it a lot easier to make sure an internal library isn't used outside of the Gem's root directory.What is the strategy for adoption?
The first step is to update the Gem Templates to expose the new
.APIINTERFACE targets with the public include directories, compile option and build dependencies.The next step is to update existing gems with the O3DE engine repo: https://github.com/o3de/o3de/tree/development/Gems with gem variant aliases for
${GemName}.<variant>.APItargets.Ideally the existing
${GemName}.Statictargets inside of gems that aren't a dependency of another gem are changed into OBJECT targets and renamed${GemName}.Private.Object.Gems with the engine which are depending on other gems
${GemName}.Statictarget should be updated to instead depend on the${GemName}.APItarget. The${GemName}.APItarget for the gem that is being depended on can be an alias of${GemName}.Staticin this case.Example: New Gem within .API targets
The following is an example of a TestGem created using the new Public/Private API split
If using the CMake Tools extension for VSCode, this is how a new Gem Targets would look
#Example: Existing Gem using an API target
The following is a contrived example of the NvCloth gem using the API target of the TestGem from the example above.
The gem maintainer of the NvCloth gem would add a build dependency on the TestGem.${API target for the runtime API target.
ly_add_target( NAME NvCloth.API INTERFACE NAMESPACE Gem FILES_CMAKE nvcloth_api_files.cmake INCLUDE_DIRECTORIES INTERFACE Include BUILD_DEPENDENCIES INTERFACE 3rdParty::NvCloth AZ::AzFramework Gem::AtomLyIntegration_CommonFeatures.API Gem::TestGem.API Gem::EMotionFX.API )Other Concerns
The O3DE repo currently contains 80 top level gems with an AutomatedTesting project which contains a single gem.
The Atom Gem contains several sub gems(Atom_RHI, Atom_RPI, etc...) as well which pushes the total number of gems to over 100.
The list of targets in gems vary based on the needs of that gem what it wishes to expose. Some gems have 0 targets others have 2~3 targets based on if Testing is enabled, while others might have 9+ targets that will show up in an IDE such as Visual Studio.
Gems are plugins, that will often have disjoint authors, that should not need to worry about any issues with a particular when authoring their gems.
For example a gem author who creates a Particle system gem that has 10 targets and host it as part of an external repo, does not need to worry if their gem can load within the engine's build solution, as only gems registered with the the current project and the engine have their CMakeLists.txt visited.
But someone adding a new gem within the engine repo, will have that gem's targets be part of the generated build solution for whatever generator was used(Ninja, Xcode, Unix Makefiles, etc...).
A subset of gems registered within the engine and resides within the O3DE repo should be moved to another repo, since they are not core to the engine itself.
The following is a list of gems that might not be core to the engine itself can be moved to another repo
Gem List
That would leave the engine repo with set of "core" gems
That would reduce the number of gems that come with the engine from 80 to 32.
The text was updated successfully, but these errors were encountered: