Skip to content

Commit

Permalink
Cleanup CPU group array info code (#369)
Browse files Browse the repository at this point in the history
  • Loading branch information
waahm7 authored Nov 13, 2023
1 parent 89791b1 commit 83008e5
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 37 deletions.
24 changes: 6 additions & 18 deletions source/s3_platform_info.c
Original file line number Diff line number Diff line change
Expand Up @@ -289,9 +289,6 @@ static void s_destroy_loader(void *arg) {
aws_hash_table_clean_up(&loader->lock_data.compute_platform_info_table);
aws_mutex_clean_up(&loader->lock_data.lock);

/* clean up the memory we allocated in init() */
aws_mem_release(loader->allocator, loader->lock_data.current_env_platform_info.cpu_group_info_array);

if (loader->lock_data.detected_instance_type) {
aws_string_destroy(loader->lock_data.detected_instance_type);
}
Expand All @@ -310,21 +307,12 @@ struct aws_s3_platform_info_loader *aws_s3_platform_info_loader_new(struct aws_a
aws_mutex_init(&loader->lock_data.lock);
aws_ref_count_init(&loader->ref_count, loader, s_destroy_loader);

/* we won't know an instance type, possibly ever, but it will be set if available before returning to the user. */
loader->lock_data.current_env_platform_info.has_recommended_configuration = false;
loader->lock_data.current_env_platform_info.cpu_group_info_array_length =
aws_system_environment_get_cpu_group_count(loader->current_env);
loader->lock_data.current_env_platform_info.cpu_group_info_array = aws_mem_calloc(
allocator,
loader->lock_data.current_env_platform_info.cpu_group_info_array_length,
sizeof(struct aws_s3_cpu_group_info));

for (size_t i = 0; i < loader->lock_data.current_env_platform_info.cpu_group_info_array_length; ++i) {
struct aws_s3_cpu_group_info *group_info = &loader->lock_data.current_env_platform_info.cpu_group_info_array[i];
group_info->cpu_group = (uint16_t)i;
group_info->cpus_in_group = aws_get_cpu_count_for_group((uint16_t)i);
/* when we have the ability to detect NIC affinity add that here. */
}
/* TODO: Implement runtime CPU information retrieval from the system. Currently, Valgrind detects a memory leak
* associated with the g_numa_node_of_cpu_ptr function (see: https://github.com/numactl/numactl/issues/3). This
* issue was addressed in version v2.0.13 of libnuma (see: https://github.com/numactl/numactl/pull/43). However,
* Amazon Linux 2 defaults to libnuma version v2.0.9, which lacks this fix. We need to suppress this
* warning as a false positive in older versions of libnuma. In the future, however, we will probably eliminate the
* use of numactl altogether. */

AWS_FATAL_ASSERT(
!aws_hash_table_init(
Expand Down
19 changes: 0 additions & 19 deletions tests/s3_platform_info_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,6 @@ static int s_test_get_existing_platform_info(struct aws_allocator *allocator, vo
ASSERT_BIN_ARRAYS_EQUALS(
instance_type.ptr, instance_type.len, platform_info->instance_type.ptr, platform_info->instance_type.len);
ASSERT_UINT_EQUALS(100, (uintmax_t)platform_info->max_throughput_gbps);
ASSERT_UINT_EQUALS(2, (uintmax_t)platform_info->cpu_group_info_array_length);
ASSERT_NOT_NULL(platform_info->cpu_group_info_array);
ASSERT_UINT_EQUALS(0, (uintmax_t)platform_info->cpu_group_info_array[0].cpu_group);
ASSERT_NOT_NULL(platform_info->cpu_group_info_array[0].nic_name_array);
ASSERT_UINT_EQUALS(1, (uintmax_t)platform_info->cpu_group_info_array[0].nic_name_array_length);

struct aws_byte_cursor nic_name = aws_byte_cursor_from_c_str("eth0");
ASSERT_BIN_ARRAYS_EQUALS(
nic_name.ptr,
nic_name.len,
platform_info->cpu_group_info_array[0].nic_name_array[0].ptr,
platform_info->cpu_group_info_array[0].nic_name_array[0].len);

ASSERT_UINT_EQUALS(1, platform_info->cpu_group_info_array[1].cpu_group);
ASSERT_NULL(platform_info->cpu_group_info_array[1].nic_name_array);
ASSERT_UINT_EQUALS(0, platform_info->cpu_group_info_array[1].nic_name_array_length);

aws_s3_platform_info_loader_release(loader);
aws_s3_library_clean_up();
Expand Down Expand Up @@ -72,8 +56,6 @@ static int s_load_platform_info_from_global_state_sanity_test(struct aws_allocat

const struct aws_s3_platform_info *platform_info = aws_s3_get_current_platform_info();
ASSERT_NOT_NULL(platform_info);
ASSERT_NOT_NULL(platform_info->cpu_group_info_array);
ASSERT_TRUE(platform_info->cpu_group_info_array_length > 0);

if (platform_info->instance_type.len) {
struct aws_s3_platform_info_loader *loader = aws_s3_platform_info_loader_new(allocator);
Expand All @@ -85,7 +67,6 @@ static int s_load_platform_info_from_global_state_sanity_test(struct aws_allocat
platform_info->instance_type.len,
by_name_info->instance_type.ptr,
by_name_info->instance_type.len);
ASSERT_UINT_EQUALS(platform_info->cpu_group_info_array_length, by_name_info->cpu_group_info_array_length);
ASSERT_TRUE(platform_info->max_throughput_gbps == by_name_info->max_throughput_gbps);
}

Expand Down

0 comments on commit 83008e5

Please sign in to comment.