Skip to content

Commit c768326

Browse files
author
Krish
committed
use locks instead of atomic var
1 parent 0506aa6 commit c768326

File tree

5 files changed

+35
-15
lines changed

5 files changed

+35
-15
lines changed

include/aws/s3/private/s3_client_impl.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,8 @@ struct aws_s3_client {
376376
struct aws_atomic_var num_requests_streaming_request_body;
377377

378378
/* Total weight of all active meta requests. Weight = object_size / (part_size * part_size) */
379-
struct aws_atomic_var total_weight;
379+
double total_weight;
380+
struct aws_mutex total_weight_mutex;
380381

381382
/* Sum of connection requirements from all active meta requests */
382383
struct aws_atomic_var total_required_connections;

source/s3_auto_ranged_get.c

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,16 @@ struct aws_s3_meta_request *aws_s3_meta_request_auto_ranged_get_new(
118118
if (options->object_size_hint != NULL) {
119119
auto_ranged_get->object_size_hint_available = true;
120120
auto_ranged_get->object_size_hint = *options->object_size_hint;
121+
122+
/* Calculate weight early if object size hint is available: weight = object_size / (part_size * part_size) */
123+
if (auto_ranged_get->object_size_hint > 0 && auto_ranged_get->base.part_size > 0) {
124+
auto_ranged_get->base.weight =
125+
(double)auto_ranged_get->object_size_hint /
126+
((double)auto_ranged_get->base.part_size * (double)auto_ranged_get->base.part_size);
127+
aws_mutex_lock(&client->stats.total_weight_mutex);
128+
client->stats.total_weight += auto_ranged_get->base.weight;
129+
aws_mutex_unlock(&client->stats.total_weight_mutex);
130+
}
121131
}
122132
AWS_LOGF_DEBUG(
123133
AWS_LS_S3_META_REQUEST, "id=%p Created new Auto-Ranged Get Meta Request.", (void *)&auto_ranged_get->base);
@@ -969,11 +979,14 @@ static void s_s3_auto_ranged_get_request_finished(
969979
}
970980

971981
/* Calculate weight for download: weight = object_size / (part_size * part_size) */
972-
if (object_size > 0 && meta_request->part_size > 0) {
982+
/* Only calculate if weight wasn't already set from object_size_hint */
983+
if (object_size > 0 && meta_request->part_size > 0 && meta_request->weight == 0.0) {
973984
meta_request->weight =
974985
(double)object_size / ((double)meta_request->part_size * (double)meta_request->part_size);
975986
/* Add this meta request's weight to client's total weight */
976-
aws_atomic_fetch_add(&meta_request->client->stats.total_weight, (size_t)meta_request->weight);
987+
aws_mutex_lock(&meta_request->client->stats.total_weight_mutex);
988+
meta_request->client->stats.total_weight += meta_request->weight;
989+
aws_mutex_unlock(&meta_request->client->stats.total_weight_mutex);
977990
}
978991
}
979992

source/s3_auto_ranged_put.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,9 @@ struct aws_s3_meta_request *aws_s3_meta_request_auto_ranged_put_new(
371371
if (has_content_length && part_size > 0) {
372372
auto_ranged_put->base.weight = (double)content_length / ((double)part_size * (double)part_size);
373373
/* Add this meta request's weight to client's total weight */
374-
aws_atomic_fetch_add(&client->stats.total_weight, (size_t)auto_ranged_put->base.weight);
374+
aws_mutex_lock(&client->stats.total_weight_mutex);
375+
client->stats.total_weight += auto_ranged_put->base.weight;
376+
aws_mutex_unlock(&client->stats.total_weight_mutex);
375377
} else {
376378
auto_ranged_put->base.weight = 0.0;
377379
}

source/s3_client.c

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -628,7 +628,8 @@ struct aws_s3_client *aws_s3_client_new(
628628

629629
aws_atomic_init_int(&client->stats.num_requests_stream_queued_waiting, 0);
630630
aws_atomic_init_int(&client->stats.num_requests_streaming_response, 0);
631-
aws_atomic_init_int(&client->stats.total_weight, 0);
631+
client->stats.total_weight = 0.0;
632+
aws_mutex_init(&client->stats.total_weight_mutex);
632633
aws_atomic_init_int(&client->stats.total_required_connections, 0);
633634
aws_atomic_init_int(&client->stats.cached_max_connections, 0);
634635
aws_atomic_init_int(&client->stats.last_max_connections_update_ns, 0);
@@ -862,6 +863,7 @@ struct aws_s3_client *aws_s3_client_new(
862863
}
863864
aws_client_bootstrap_release(client->client_bootstrap);
864865
aws_mutex_clean_up(&client->synced_data.lock);
866+
aws_mutex_clean_up(&client->stats.total_weight_mutex);
865867

866868
aws_mem_release(client->allocator, client->network_interface_names_cursor_array);
867869
for (size_t i = 0; i < aws_array_list_length(&client->network_interface_names); i++) {
@@ -966,6 +968,7 @@ static void s_s3_client_finish_destroy_default(struct aws_s3_client *client) {
966968
aws_mem_release(client->allocator, client->tcp_keep_alive_options);
967969

968970
aws_mutex_clean_up(&client->synced_data.lock);
971+
aws_mutex_clean_up(&client->stats.total_weight_mutex);
969972

970973
AWS_ASSERT(aws_linked_list_empty(&client->synced_data.pending_meta_request_work));
971974
AWS_ASSERT(aws_linked_list_empty(&client->threaded_data.meta_requests));
@@ -2369,6 +2372,7 @@ void aws_s3_client_update_connections_threaded(struct aws_s3_client *client) {
23692372

23702373
struct aws_s3_request *request = aws_s3_client_dequeue_request_threaded(client);
23712374
struct aws_s3_meta_request *meta_request = request->meta_request;
2375+
const uint32_t max_active_connections = aws_s3_client_get_max_active_connections(client, meta_request);
23722376
/* As the request removed from the queue. Decrement the preparing track */
23732377
--meta_request->client_process_work_threaded_data.num_request_being_prepared;
23742378
if (request->is_noop) {
@@ -2395,18 +2399,16 @@ void aws_s3_client_update_connections_threaded(struct aws_s3_client *client) {
23952399
bool should_allocate_connection = false;
23962400
uint32_t current_connections = (uint32_t)aws_atomic_load_int(&meta_request->num_requests_network);
23972401

2398-
if (meta_request->weight > 0.0) {
2399-
size_t total_weight = aws_atomic_load_int(&client->stats.total_weight);
2400-
if (total_weight > 0) {
2402+
if (meta_request->weight > (double)0) {
2403+
aws_mutex_lock(&client->stats.total_weight_mutex);
2404+
double total_weight = client->stats.total_weight;
2405+
aws_mutex_unlock(&client->stats.total_weight_mutex);
2406+
if (total_weight > (double)0) {
24012407
/* Calculate: (meta_request->weight / client->total_weight) * 300 */
2402-
double weight_ratio = meta_request->weight / (double)total_weight;
2408+
double weight_ratio = meta_request->weight / total_weight;
24032409
uint32_t allowed_connections = (uint32_t)(weight_ratio * client_max_active_connections);
24042410

2405-
/* Apply max_active_connections_override if set */
2406-
if (meta_request->max_active_connections_override > 0) {
2407-
allowed_connections =
2408-
aws_min_u32(allowed_connections, meta_request->max_active_connections_override);
2409-
}
2411+
allowed_connections = aws_min_u32(allowed_connections, max_active_connections);
24102412

24112413
should_allocate_connection = (current_connections < allowed_connections);
24122414
} else {

source/s3_meta_request.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -615,7 +615,9 @@ static void s_s3_meta_request_destroy(void *user_data) {
615615
if (meta_request->client != NULL) {
616616
/* Subtract this meta request's weight from client's total weight */
617617
if (meta_request->weight > 0.0) {
618-
aws_atomic_fetch_sub(&meta_request->client->stats.total_weight, (size_t)meta_request->weight);
618+
aws_mutex_lock(&meta_request->client->stats.total_weight_mutex);
619+
meta_request->client->stats.total_weight -= meta_request->weight;
620+
aws_mutex_unlock(&meta_request->client->stats.total_weight_mutex);
619621
}
620622

621623
/* Subtract this meta request's connection requirements from client's total */

0 commit comments

Comments
 (0)