Skip to content

Commit eb68ebb

Browse files
committed
RRDP Notification: Optimize delta parse
It was allocating the deltas array twice, for seemingly no reason. Also, the array slots were pointers, and the two arrays pointed to different instances of the same objects. For seemingly no reason. Now there's only one array, and it stores the objects directly. Also adds relevant unit tests.
1 parent 33664f3 commit eb68ebb

File tree

6 files changed

+243
-352
lines changed

6 files changed

+243
-352
lines changed

Diff for: src/rrdp/rrdp_objects.c

+73-191
Original file line numberDiff line numberDiff line change
@@ -5,26 +5,7 @@
55
#include <string.h>
66
#include "log.h"
77

8-
/*
9-
* List of deltas inside an update notification file.
10-
*
11-
* The structure functions are extended and will have the following meaning:
12-
* - capacity : is the size of the array, must be set before using the array
13-
* and can't be modified.
14-
* - len : number of elements set in the array.
15-
*
16-
* This struct is a diff version of array_list, utilized to store only the
17-
* amount of deltas that may be needed and validate that an update notification
18-
* file has a contiguous set of deltas.
19-
*/
20-
struct deltas_head {
21-
/** Unidimensional array. Initialized lazily. */
22-
struct delta_head **array;
23-
/** Number of elements in @array. */
24-
size_t len;
25-
/** Actual allocated slots in @array. */
26-
size_t capacity;
27-
};
8+
DEFINE_ARRAY_LIST_FUNCTIONS(deltas_head, struct delta_head, )
289

2910
void
3011
global_data_init(struct global_data *data)
@@ -41,9 +22,9 @@ global_data_cleanup(struct global_data *data)
4122
void
4223
doc_data_init(struct doc_data *data)
4324
{
25+
data->uri = NULL;
4426
data->hash = NULL;
4527
data->hash_len = 0;
46-
data->uri = NULL;
4728
}
4829

4930
void
@@ -53,157 +34,6 @@ doc_data_cleanup(struct doc_data *data)
5334
free(data->uri);
5435
}
5536

56-
int
57-
delta_head_create(struct delta_head **result)
58-
{
59-
struct delta_head *tmp;
60-
61-
tmp = malloc(sizeof(struct delta_head));
62-
if (tmp == NULL)
63-
return pr_enomem();
64-
65-
doc_data_init(&tmp->doc_data);
66-
67-
*result = tmp;
68-
return 0;
69-
}
70-
71-
void
72-
delta_head_destroy(struct delta_head *delta_head)
73-
{
74-
if (delta_head) {
75-
doc_data_cleanup(&delta_head->doc_data);
76-
free(delta_head);
77-
}
78-
}
79-
80-
static void
81-
deltas_head_init(struct deltas_head *list)
82-
{
83-
list->array = NULL;
84-
list->len = 0;
85-
list->capacity = 0;
86-
}
87-
88-
static void
89-
deltas_head_cleanup(struct deltas_head *list)
90-
{
91-
size_t i;
92-
93-
for (i = 0; i < list->capacity; i++)
94-
delta_head_destroy(list->array[i]);
95-
if (list->array)
96-
free(list->array);
97-
}
98-
99-
static int
100-
deltas_head_create(struct deltas_head **deltas)
101-
{
102-
struct deltas_head *tmp;
103-
104-
tmp = malloc(sizeof(struct deltas_head));
105-
if (tmp == NULL)
106-
return pr_enomem();
107-
108-
deltas_head_init(tmp);
109-
110-
*deltas = tmp;
111-
return 0;
112-
}
113-
114-
static void
115-
deltas_head_destroy(struct deltas_head *deltas)
116-
{
117-
deltas_head_cleanup(deltas);
118-
free(deltas);
119-
}
120-
121-
int
122-
deltas_head_set_size(struct deltas_head *deltas, size_t capacity)
123-
{
124-
size_t i;
125-
126-
if (deltas->array != NULL)
127-
pr_crit("Size of this list can't be modified");
128-
129-
deltas->capacity = capacity;
130-
if (capacity == 0)
131-
return 0; /* Ok, list can have 0 elements */
132-
133-
deltas->array = malloc(deltas->capacity
134-
* sizeof(struct delta_head *));
135-
if (deltas->array == NULL)
136-
return pr_enomem();
137-
138-
/* Point all elements to NULL */
139-
for (i = 0; i < deltas->capacity; i++)
140-
deltas->array[i] = NULL;
141-
142-
return 0;
143-
}
144-
145-
/*
146-
* A new delta_head will be allocated at its corresponding position inside
147-
* @deltas (also its URI and HASH will be allocated). The position is calculated
148-
* using the difference between @max_serial and @serial.
149-
*
150-
* The following errors can be returned due to a wrong @position:
151-
* -EEXIST: There's already an element at @position.
152-
* -EINVAL: @position can't be inside @deltas list, meaning that such element
153-
* isn't part of a contiguous list.
154-
*
155-
* Don't forget to call deltas_head_set_size() before this!!
156-
*/
157-
int
158-
deltas_head_add(struct deltas_head *deltas, unsigned long max_serial,
159-
unsigned long serial, char *uri, unsigned char *hash, size_t hash_len)
160-
{
161-
struct delta_head *elem;
162-
size_t position;
163-
int error;
164-
165-
position = deltas->capacity - 1 - (max_serial - serial);
166-
if (position < 0 || position > deltas->capacity - 1)
167-
return -EINVAL;
168-
169-
if (deltas->array[position] != NULL)
170-
return -EEXIST;
171-
172-
elem = NULL;
173-
error = delta_head_create(&elem);
174-
if (error)
175-
return error;
176-
177-
elem->serial = serial;
178-
179-
elem->doc_data.uri = strdup(uri);
180-
if (elem->doc_data.uri == NULL) {
181-
free(elem);
182-
return pr_enomem();
183-
}
184-
185-
elem->doc_data.hash_len = hash_len;
186-
elem->doc_data.hash = malloc(hash_len);
187-
if (elem->doc_data.hash == NULL) {
188-
free(elem->doc_data.uri);
189-
free(elem);
190-
return pr_enomem();
191-
}
192-
memcpy(elem->doc_data.hash, hash, hash_len);
193-
194-
deltas->array[position] = elem;
195-
deltas->len++;
196-
197-
return 0;
198-
}
199-
200-
/* Are all expected values set? */
201-
bool
202-
deltas_head_values_set(struct deltas_head *deltas)
203-
{
204-
return deltas->len == deltas->capacity;
205-
}
206-
20737
/* Do the @cb to the delta head elements from @from_serial to @max_serial */
20838
int
20939
deltas_head_for_each(struct deltas_head *deltas, unsigned long max_serial,
@@ -223,47 +53,99 @@ deltas_head_for_each(struct deltas_head *deltas, unsigned long max_serial,
22353
max_serial);
22454
from = deltas->capacity - (max_serial - from_serial);
22555
for (index = from; index < deltas->capacity; index++) {
226-
error = cb(deltas->array[index], arg);
56+
error = cb(&deltas->array[index], arg);
22757
if (error)
22858
return error;
22959
}
23060

23161
return 0;
23262
}
23363

64+
static int
65+
swap_until_sorted(struct delta_head *deltas, unsigned int i,
66+
unsigned long min, unsigned long max)
67+
{
68+
unsigned int target_slot;
69+
struct delta_head tmp;
70+
71+
while (true) {
72+
if (deltas[i].serial < min || max < deltas[i].serial) {
73+
return pr_val_err("Deltas: Serial '%lu' is out of bounds. (min:%lu, max:%lu)",
74+
deltas[i].serial, min, max);
75+
}
76+
77+
target_slot = deltas[i].serial - min;
78+
if (i == target_slot)
79+
return 0;
80+
if (deltas[target_slot].serial == deltas[i].serial) {
81+
return pr_val_err("Deltas: Serial '%lu' is not unique.",
82+
deltas[i].serial);
83+
}
84+
85+
/* Simple swap */
86+
tmp = deltas[target_slot];
87+
deltas[target_slot] = deltas[i];
88+
deltas[i] = tmp;
89+
}
90+
}
91+
23492
int
235-
update_notification_create(struct update_notification **file)
93+
deltas_head_sort(struct deltas_head *deltas, unsigned long max_serial)
23694
{
237-
struct update_notification *tmp;
238-
struct deltas_head *list;
95+
unsigned long min_serial;
96+
struct delta_head *cursor;
97+
array_index i;
23998
int error;
24099

241-
tmp = malloc(sizeof(struct update_notification));
242-
if (tmp == NULL)
243-
return pr_enomem();
100+
if (max_serial + 1 < deltas->len)
101+
return pr_val_err("Deltas: Too many deltas (%zu) for serial %lu. (Negative serials not implemented.)",
102+
deltas->len, max_serial);
244103

245-
list = NULL;
246-
error = deltas_head_create(&list);
247-
if (error) {
248-
free(tmp);
249-
return error;
250-
}
251-
tmp->deltas_list = list;
252-
tmp->uri = NULL;
104+
min_serial = max_serial + 1 - deltas->len;
253105

254-
global_data_init(&tmp->global_data);
255-
doc_data_init(&tmp->snapshot);
106+
ARRAYLIST_FOREACH(deltas, cursor, i) {
107+
error = swap_until_sorted(deltas->array, i, min_serial,
108+
max_serial);
109+
if (error)
110+
return error;
111+
}
256112

257-
*file = tmp;
258113
return 0;
259114
}
260115

116+
struct update_notification *
117+
update_notification_create(char const *uri)
118+
{
119+
struct update_notification *result;
120+
121+
result = malloc(sizeof(struct update_notification));
122+
if (result == NULL)
123+
return NULL;
124+
125+
global_data_init(&result->global_data);
126+
doc_data_init(&result->snapshot);
127+
deltas_head_init(&result->deltas_list);
128+
result->uri = strdup(uri);
129+
if (result->uri == NULL) {
130+
free(result);
131+
return NULL;
132+
}
133+
134+
return result;
135+
}
136+
137+
static void
138+
delta_head_destroy(struct delta_head *delta)
139+
{
140+
doc_data_cleanup(&delta->doc_data);
141+
}
142+
261143
void
262144
update_notification_destroy(struct update_notification *file)
263145
{
264146
doc_data_cleanup(&file->snapshot);
265147
global_data_cleanup(&file->global_data);
266-
deltas_head_destroy(file->deltas_list);
148+
deltas_head_cleanup(&file->deltas_list, delta_head_destroy);
267149
free(file->uri);
268150
free(file);
269151
}

Diff for: src/rrdp/rrdp_objects.h

+10-11
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
#include <stddef.h>
55
#include <stdbool.h>
6+
#include "data_structure/array_list.h"
67

78
/* Possible results for an RRDP URI comparison */
89
typedef enum {
@@ -62,18 +63,23 @@ struct snapshot {
6263

6364
/* Delta element located at an update notification file */
6465
struct delta_head {
66+
/*
67+
* TODO this is not an RFC 1982 serial. It's supposed to be unbounded,
68+
* so we should probably handle it as a string.
69+
*/
6570
unsigned long serial;
6671
struct doc_data doc_data;
6772
};
6873

6974
/* List of deltas inside an update notification file */
70-
struct deltas_head;
75+
DEFINE_ARRAY_LIST_STRUCT(deltas_head, struct delta_head);
76+
DECLARE_ARRAY_LIST_FUNCTIONS(deltas_head, struct delta_head)
7177

7278
/* Update notification file content and location URI */
7379
struct update_notification {
7480
struct global_data global_data;
7581
struct doc_data snapshot;
76-
struct deltas_head *deltas_list;
82+
struct deltas_head deltas_list;
7783
char *uri;
7884
};
7985

@@ -83,20 +89,13 @@ void global_data_cleanup(struct global_data *);
8389
void doc_data_init(struct doc_data *);
8490
void doc_data_cleanup(struct doc_data *);
8591

86-
int update_notification_create(struct update_notification **);
92+
struct update_notification *update_notification_create(char const *);
8793
void update_notification_destroy(struct update_notification *);
8894

89-
int delta_head_create(struct delta_head **);
90-
void delta_head_destroy(struct delta_head *);
91-
9295
typedef int (*delta_head_cb)(struct delta_head *, void *);
9396
int deltas_head_for_each(struct deltas_head *, unsigned long, unsigned long,
9497
delta_head_cb, void *);
95-
int deltas_head_add(struct deltas_head *, unsigned long, unsigned long, char *,
96-
unsigned char *, size_t);
97-
98-
int deltas_head_set_size(struct deltas_head *, size_t);
99-
bool deltas_head_values_set(struct deltas_head *);
98+
int deltas_head_sort(struct deltas_head *, unsigned long);
10099

101100
int snapshot_create(struct snapshot **);
102101
void snapshot_destroy(struct snapshot *);

0 commit comments

Comments
 (0)