Skip to content

Commit e282048

Browse files
committed
[CoreCLR] use string blob for environment variables
Eliminates N*2 relocations
1 parent 1bf8547 commit e282048

File tree

4 files changed

+77
-15
lines changed

4 files changed

+77
-15
lines changed

src/Xamarin.Android.Build.Tasks/Utilities/ApplicationConfigNativeAssemblyGeneratorCLR.cs

Lines changed: 63 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,42 @@ sealed class RuntimePropertyIndexEntry
134134
public uint index;
135135
}
136136

137+
sealed class AppEnvironmentVariableContextDataProvider : NativeAssemblerStructContextDataProvider
138+
{
139+
public override string GetComment (object data, string fieldName)
140+
{
141+
var envVar = EnsureType<AppEnvironmentVariable> (data);
142+
143+
if (String.Compare ("name_index", fieldName, StringComparison.Ordinal) == 0) {
144+
return $" '{envVar.Name}'";
145+
}
146+
147+
if (String.Compare ("value_index", fieldName, StringComparison.Ordinal) == 0) {
148+
return $" '{envVar.Value}'";
149+
}
150+
151+
return String.Empty;
152+
}
153+
}
154+
155+
// Order of fields and their type must correspond *exactly* to that in
156+
// src/native/clr/include/xamarin-app.hh AppEnvironmentVariable structure
157+
[NativeAssemblerStructContextDataProvider (typeof (AppEnvironmentVariableContextDataProvider))]
158+
sealed class AppEnvironmentVariable
159+
{
160+
[NativeAssembler (Ignore = true)]
161+
public string Name;
162+
163+
[NativeAssembler (Ignore = true)]
164+
public string Value;
165+
166+
[NativeAssembler (UsesDataProvider = true)]
167+
public uint name_index;
168+
169+
[NativeAssembler (UsesDataProvider = true)]
170+
public uint value_index;
171+
}
172+
137173
sealed class XamarinAndroidBundledAssemblyContextDataProvider : NativeAssemblerStructContextDataProvider
138174
{
139175
public override ulong GetBufferSize (object data, string fieldName)
@@ -195,6 +231,7 @@ sealed class XamarinAndroidBundledAssembly
195231
StructureInfo? runtimePropertyIndexEntryStructureInfo;
196232
StructureInfo? hostConfigurationPropertyStructureInfo;
197233
StructureInfo? hostConfigurationPropertiesStructureInfo;
234+
StructureInfo? appEnvironmentVariableStructureInfo;
198235

199236
public bool UsesAssemblyPreload { get; set; }
200237
public string AndroidPackageName { get; set; }
@@ -244,10 +281,32 @@ protected override void Construct (LlvmIrModule module)
244281

245282
module.AddGlobalVariable ("format_tag", FORMAT_TAG, comment: $" 0x{FORMAT_TAG:x}");
246283

247-
var envVars = new LlvmIrGlobalVariable (environmentVariables, "app_environment_variables") {
284+
var envVarsBlob = new LlvmIrStringBlob ();
285+
var appEnvVars = new List<StructureInstance<AppEnvironmentVariable>> ();
286+
287+
if (environmentVariables != null) {
288+
// TODO: skip variables with no name
289+
foreach (var kvp in environmentVariables) {
290+
(int nameOffset, int _) = envVarsBlob.Add (kvp.Key);
291+
(int valueOffset, int _) = envVarsBlob.Add (kvp.Value);
292+
293+
var appEnvVar = new AppEnvironmentVariable {
294+
Name = kvp.Key,
295+
Value = kvp.Value,
296+
297+
name_index = (uint)nameOffset,
298+
value_index = (uint)valueOffset,
299+
};
300+
appEnvVars.Add (new StructureInstance<AppEnvironmentVariable> (appEnvironmentVariableStructureInfo, appEnvVar));
301+
}
302+
}
303+
304+
var envVars = new LlvmIrGlobalVariable (appEnvVars, "app_environment_variables") {
248305
Comment = " Application environment variables array, name:value",
306+
Options = LlvmIrVariableOptions.GlobalConstant,
249307
};
250-
module.Add (envVars, stringGroupName: "env", stringGroupComment: " Application environment variables name:value pairs");
308+
module.Add (envVars);
309+
module.AddGlobalVariable ("app_environment_variable_contents", envVarsBlob, LlvmIrVariableOptions.GlobalConstant);
251310

252311
var sysProps = new LlvmIrGlobalVariable (systemProperties, "app_system_properties") {
253312
Comment = " System properties defined by the application",
@@ -263,7 +322,7 @@ protected override void Construct (LlvmIrModule module)
263322
ignore_split_configs = IgnoreSplitConfigs,
264323
number_of_runtime_properties = (uint)(runtimeProperties == null ? 0 : runtimeProperties.Count),
265324
package_naming_policy = (uint)PackageNamingPolicy,
266-
environment_variable_count = (uint)(environmentVariables == null ? 0 : environmentVariables.Count * 2),
325+
environment_variable_count = (uint)(environmentVariables == null ? 0 : environmentVariables.Count),
267326
system_property_count = (uint)(systemProperties == null ? 0 : systemProperties.Count * 2),
268327
number_of_assemblies_in_apk = (uint)NumberOfAssembliesInApk,
269328
number_of_shared_libraries = (uint)NativeLibraries.Count,
@@ -541,5 +600,6 @@ void MapStructures (LlvmIrModule module)
541600
dsoApkEntryStructureInfo = module.MapStructure<DSOApkEntry> ();
542601
runtimePropertyStructureInfo = module.MapStructure<RuntimeProperty> ();
543602
runtimePropertyIndexEntryStructureInfo = module.MapStructure<RuntimePropertyIndexEntry> ();
603+
appEnvironmentVariableStructureInfo = module.MapStructure<AppEnvironmentVariable> ();
544604
}
545605
}

src/native/clr/include/xamarin-app.hh

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,12 @@ struct JniRemappingTypeReplacementEntry
322322
const char *replacement;
323323
};
324324

325+
struct AppEnvironmentVariable
326+
{
327+
const uint32_t name_index;
328+
const uint32_t value_index;
329+
};
330+
325331
extern "C" {
326332
[[gnu::visibility("default")]] extern const JniRemappingIndexTypeEntry jni_remapping_method_replacement_index[];
327333
[[gnu::visibility("default")]] extern const JniRemappingTypeReplacementEntry jni_remapping_type_replacements[];
@@ -353,7 +359,8 @@ extern "C" {
353359
[[gnu::visibility("default")]] extern uint32_t uncompressed_assemblies_data_size;
354360
[[gnu::visibility("default")]] extern uint8_t uncompressed_assemblies_data_buffer[];
355361
[[gnu::visibility("default")]] extern const ApplicationConfig application_config;
356-
[[gnu::visibility("default")]] extern const char* const app_environment_variables[];
362+
[[gnu::visibility("default")]] extern const AppEnvironmentVariable app_environment_variables[];
363+
[[gnu::visibility("default")]] extern const char app_environment_variable_contents[];
357364
[[gnu::visibility("default")]] extern const char* const app_system_properties[];
358365

359366
[[gnu::visibility("default")]] extern const char* const mono_aot_mode_name;

src/native/clr/runtime-base/android-system.cc

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -230,16 +230,10 @@ AndroidSystem::setup_environment () noexcept
230230

231231
const char *var_name;
232232
const char *var_value;
233-
for (size_t i = 0uz; i < application_config.environment_variable_count; i += 2) {
234-
var_name = app_environment_variables [i];
235-
if (var_name == nullptr || *var_name == '\0') {
236-
continue;
237-
}
238-
239-
var_value = app_environment_variables [i + 1uz];
240-
if (var_value == nullptr) {
241-
var_value = "";
242-
}
233+
for (size_t i = 0uz; i < application_config.environment_variable_count; i++) {
234+
AppEnvironmentVariable const& env_var = app_environment_variables [i];
235+
var_name = &app_environment_variable_contents[env_var.name_index];
236+
var_value = &app_environment_variable_contents[env_var.value_index];
243237

244238
if constexpr (Constants::is_debug_build) {
245239
log_info (LOG_DEFAULT, "Setting environment variable '{}' to '{}'", var_name, var_value);

src/native/clr/xamarin-app-stub/application_dso_stub.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,8 @@ const ApplicationConfig application_config = {
7272
};
7373

7474
// TODO: migrate to std::string_view for these two
75-
const char* const app_environment_variables[] = {};
75+
const AppEnvironmentVariable app_environment_variables[] = {};
76+
const char app_environment_variable_contents[] = {};
7677
const char* const app_system_properties[] = {};
7778

7879
static constexpr size_t AssemblyNameWidth = 128uz;

0 commit comments

Comments
 (0)