Skip to content

Commit 3bbd303

Browse files
authoredMay 7, 2020
Simplify data embedding (#270)
This is an internal-only refactoring that changes the mechanics of how the sdf/** file data is embedded into the library. For one, it solves the "static initialization order fiasco" for the embedded data. The data is only loaded into memory upon first use, instead of as the library is being loaded. It also simplifies the ruby embedding code, making it more concise. I hope this is easier to maintain, but it also helps me when sharing Drake with consumers that always build from source, but will not install ruby as a compile-time prerequisite The EmbeddedSdf codegen now emits the cc file with the data; the header is stored in git. This provides an easier way to use a function to retrieve the embedded strings as static locals. The embedded SDF data is now combined into a single map. Embedding files should be boring, ala the WIP std::embed specification. It should not include application-layer logic encoding upgrade paths. This will also make it easier to avoid the "static destruction fiasco" in future commits, due to fewer functions to repair. Signed-off-by: Jeremy Nimmer <jeremy.nimmer@tri.global>
1 parent 9eb6b99 commit 3bbd303

File tree

6 files changed

+109
-79
lines changed

6 files changed

+109
-79
lines changed
 

‎sdf/CMakeLists.txt

+2-2
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,12 @@ add_subdirectory(1.8)
1010
add_custom_target(schema)
1111
add_dependencies(schema schema1_8)
1212

13-
# Generate the EmbeddedSdf.hh file, which contains all the supported SDF
13+
# Generate the EmbeddedSdf.cc file, which contains all the supported SDF
1414
# descriptions in a map of strings. The parser.cc file uses EmbeddedSdf.hh.
1515
execute_process(
1616
COMMAND ${RUBY} ${CMAKE_SOURCE_DIR}/sdf/embedSdf.rb
1717
WORKING_DIRECTORY "${CMAKE_SOURCE_DIR}/sdf"
18-
OUTPUT_FILE "${PROJECT_BINARY_DIR}/include/sdf/EmbeddedSdf.hh"
18+
OUTPUT_FILE "${PROJECT_BINARY_DIR}/src/EmbeddedSdf.cc"
1919
)
2020

2121
# Generate aggregated SDF description files for use by the sdformat.org

‎sdf/embedSdf.rb

+23-48
Original file line numberDiff line numberDiff line change
@@ -9,62 +9,37 @@
99
supportedSdfConversions = ['1.8', '1.7', '1.6', '1.5', '1.4', '1.3']
1010

1111
puts %q!
12-
#ifndef SDF_INTERNAL_EMBEDDEDSDF_HH_
13-
#define SDF_INTERNAL_EMBEDDEDSDF_HH_
12+
#include "EmbeddedSdf.hh"
1413
15-
// An empty SDF string is returned if a query into the embeddedSdf map fails.
16-
static const std::string emptySdfString = "";
14+
namespace sdf {
15+
inline namespace SDF_VERSION_NAMESPACE {
1716
18-
// A map of maps where the keys in the first/parent map are SDF version
19-
// strings, keys in the second/child map are SDF specification filenames and
20-
// values are the contents of the SDF specification files.
21-
static const std::map<std::string, std::map<std::string, std::string>> embeddedSdf = {
17+
const std::map<std::string, std::string> &GetEmbeddedSdf() {
18+
static const std::map<std::string, std::string> result{
2219
!
2320

24-
# Iterate over each version
25-
supportedSdfVersions.each do |version|
26-
# Make sure the directory exists. Quietly fail so that we don't pollute
27-
# the output, which gets included in EmbeddedSdf.hh
28-
if Dir.exist?(version)
29-
puts "{\"#{version}\", {"
30-
31-
# Iterate over each .sdf file in the version directory
32-
Dir.glob("#{version}/*.sdf") do |file|
33-
34-
# Store the contents of the file in the child map
35-
puts "{\"#{File.basename(file)}\", R\"__sdf_literal__("
36-
infile = File.open(file)
37-
puts infile.read
38-
puts ")__sdf_literal__\"},"
39-
end
40-
puts "}},"
41-
end
21+
# Stores the contents of the file in the map.
22+
def embed(pathname)
23+
puts "{\"#{pathname}\", R\"__sdf_literal__("
24+
infile = File.open(pathname)
25+
puts infile.read
26+
puts ")__sdf_literal__\"},"
4227
end
4328

44-
puts "};"
45-
46-
puts "static const std::map<std::string, std::pair<std::string, std::string>> conversionMap = {"
29+
# Embed the supported *.sdf files.
30+
supportedSdfVersions.each do |version|
31+
Dir.glob("#{version}/*.sdf").sort.each { |file| embed(file) }
32+
end
4733

48-
# Iterate over each version
34+
# Embed the supported *.convert files.
4935
supportedSdfConversions.each do |version|
50-
# from-to
51-
# Make sure the directory exists. Quietly fail so that we don't pollute
52-
# the output, which gets included in EmbeddedSdf.hh
53-
if Dir.exist?(version)
54-
55-
# Iterate over each .sdf file in the version directory
56-
Dir.glob("#{version}/*.convert") do |file|
57-
58-
basename = File.basename(file, ".*").gsub(/_/, '.')
59-
# Store the contents of the file in the child map
60-
puts "{\"#{basename}\", {\"#{version}\", R\"__sdf_literal__("
61-
infile = File.open(file)
62-
puts infile.read
63-
puts ")__sdf_literal__\"}},"
64-
end
65-
end
36+
Dir.glob("#{version}/*.convert").sort.each { |file| embed(file) }
6637
end
6738
puts %q!
68-
};
69-
#endif
39+
};
40+
return result;
41+
}
42+
43+
}
44+
}
7045
!

‎src/CMakeLists.txt

+3-1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ set (sources
2424
Converter.cc
2525
Cylinder.cc
2626
Element.cc
27+
EmbeddedSdf.cc
2728
Error.cc
2829
Exception.cc
2930
Frame.cc
@@ -62,6 +63,7 @@ set (sources
6263
Visual.cc
6364
World.cc
6465
)
66+
include_directories(${CMAKE_CURRENT_SOURCE_DIR})
6567

6668
if (USE_EXTERNAL_TINYXML)
6769
include_directories(${tinyxml_INCLUDE_DIRS})
@@ -157,7 +159,7 @@ if (NOT WIN32)
157159
endif()
158160

159161
if (NOT WIN32)
160-
set(SDF_BUILD_TESTS_EXTRA_EXE_SRCS Converter.cc)
162+
set(SDF_BUILD_TESTS_EXTRA_EXE_SRCS Converter.cc EmbeddedSdf.cc)
161163
sdf_build_tests(Converter_TEST.cc)
162164
endif()
163165

‎src/Converter.cc

+35-24
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,18 @@
2929
#include "sdf/Types.hh"
3030

3131
#include "Converter.hh"
32-
33-
// This include file is generated at configure time.
34-
#include "sdf/EmbeddedSdf.hh"
32+
#include "EmbeddedSdf.hh"
3533

3634
using namespace sdf;
3735

36+
namespace {
37+
bool EndsWith(const std::string& _a, const std::string& _b)
38+
{
39+
return (_a.size() >= _b.size()) &&
40+
(_a.compare(_a.size() - _b.size(), _b.size(), _b) == 0);
41+
}
42+
}
43+
3844
/////////////////////////////////////////////////
3945
bool Converter::Convert(TiXmlDocument *_doc, const std::string &_toVersion,
4046
bool _quiet)
@@ -73,42 +79,47 @@ bool Converter::Convert(TiXmlDocument *_doc, const std::string &_toVersion,
7379

7480
elem->SetAttribute("version", _toVersion);
7581

76-
// The conversionMap in EmbeddedSdf.hh has keys that represent a version
77-
// of SDF to convert from. The values in conversionmap are pairs, where
78-
// the first element is the SDF version that the second element will
79-
// convert to. For example, the following will convert from 1.4 to 1.5
80-
// according to "conversion_xml":
81-
//
82-
// {"1.4", {"1.5", "conversion_xml"}}
83-
std::map<std::string, std::pair<std::string, std::string> >::const_iterator
84-
fromIter = conversionMap.find(origVersion);
85-
86-
std::string toVer = "";
82+
// The conversion recipes within the embedded files database are named, e.g.,
83+
// "1.8/1_7.convert" to upgrade from 1.7 to 1.8.
84+
const std::map<std::string, std::string> &embedded = GetEmbeddedSdf();
8785

88-
// Starting with the original SDF version, perform all the conversions
89-
// necessary in order to reach the _toVersion.
90-
while (fromIter != conversionMap.end() && fromIter->first != _toVersion)
86+
// Apply the conversions one at a time until we reach the desired _toVersion.
87+
std::string curVersion = origVersion;
88+
while (curVersion != _toVersion)
9189
{
92-
// Get the SDF to version.
93-
toVer = fromIter->second.first;
90+
// Find the (at most one) file named, e.g., ".../1_7.convert".
91+
std::string snakeVersion = curVersion;
92+
std::replace(snakeVersion.begin(), snakeVersion.end(), '.', '_');
93+
const std::string suffix = "/" + snakeVersion + ".convert";
94+
const char* convertXml = nullptr;
95+
for (const auto& [pathname, data] : embedded)
96+
{
97+
if (EndsWith(pathname, suffix))
98+
{
99+
curVersion = pathname.substr(0, pathname.size() - suffix.size());
100+
convertXml = data.c_str();
101+
break;
102+
}
103+
}
104+
if (convertXml == nullptr)
105+
{
106+
break;
107+
}
94108

95109
// Parse and apply the conversion XML.
96110
TiXmlDocument xmlDoc;
97-
xmlDoc.Parse(fromIter->second.second.c_str());
111+
xmlDoc.Parse(convertXml);
98112
if (xmlDoc.Error())
99113
{
100114
sdferr << "Error parsing XML from string: "
101115
<< xmlDoc.ErrorDesc() << '\n';
102116
return false;
103117
}
104118
ConvertImpl(elem, xmlDoc.FirstChildElement("convert"));
105-
106-
// Get the next conversion XML map element.
107-
fromIter = conversionMap.find(toVer);
108119
}
109120

110121
// Check that we actually converted to the desired final version.
111-
if (toVer != _toVersion)
122+
if (curVersion != _toVersion)
112123
{
113124
sdferr << "Unable to convert from SDF version " << origVersion
114125
<< " to " << _toVersion << "\n";

‎src/EmbeddedSdf.hh

+40
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/*
2+
* Copyright 2020 Open Source Robotics Foundation
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*
16+
*/
17+
18+
#ifndef SDF_EMBEDDEDSDF_HH_
19+
#define SDF_EMBEDDEDSDF_HH_
20+
21+
#include <map>
22+
#include <string>
23+
24+
#include "sdf/Types.hh"
25+
26+
namespace sdf
27+
{
28+
// Inline bracket to help doxygen filtering.
29+
inline namespace SDF_VERSION_NAMESPACE {
30+
//
31+
32+
/// \internal
33+
34+
/// A map where the keys are a source-relative pathnames within the "sdf"
35+
/// directory such as "1.8/root.sdf", and the values are the contents of
36+
/// that source file.
37+
const std::map<std::string, std::string> &GetEmbeddedSdf();
38+
}
39+
}
40+
#endif

‎src/SDF.cc

+6-4
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,7 @@
3131
#include "sdf/SDFImpl.hh"
3232
#include "SDFImplPrivate.hh"
3333
#include "sdf/sdf_config.h"
34-
35-
// This include file is generated at configure time.
36-
#include "sdf/EmbeddedSdf.hh"
34+
#include "EmbeddedSdf.hh"
3735

3836
namespace sdf
3937
{
@@ -453,14 +451,18 @@ const std::string &SDF::EmbeddedSpec(
453451
{
454452
try
455453
{
456-
return embeddedSdf.at(SDF::Version()).at(_filename);
454+
const std::string pathname = SDF::Version() + "/" + _filename;
455+
return GetEmbeddedSdf().at(pathname);
457456
}
458457
catch(const std::out_of_range &)
459458
{
460459
if (!_quiet)
461460
sdferr << "Unable to find SDF filename[" << _filename << "] with "
462461
<< "version " << SDF::Version() << "\n";
463462
}
463+
464+
// An empty SDF string is returned if a query into the embeddedSdf map fails.
465+
static const std::string emptySdfString;
464466
return emptySdfString;
465467
}
466468
}

0 commit comments

Comments
 (0)