v2: zero the update array instead of checking the return value. SLLocate returns NULL both on failure and if the element is greater than everything in the list
Signed-off-by: Jan Vesely jan.vesely@rutgers.edu --- xf86drmSL.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/xf86drmSL.c b/xf86drmSL.c index acddb54..cf588ac 100644 --- a/xf86drmSL.c +++ b/xf86drmSL.c @@ -264,12 +264,14 @@ int drmSLLookupNeighbors(void *l, unsigned long key, unsigned long *next_key, void **next_value) { SkipListPtr list = (SkipListPtr)l; - SLEntryPtr update[SL_MAX_LEVEL + 1]; + SLEntryPtr update[SL_MAX_LEVEL + 1] = {0}; int retcode = 0;
+ SLLocate(list, key, update); + *prev_key = *next_key = key; *prev_value = *next_value = NULL; - + if (update[0]) { *prev_key = update[0]->key; *prev_value = update[0]->value;
Signed-off-by: Jan Vesely jan.vesely@rutgers.edu --- .gitignore | 1 + tests/Makefile.am | 3 +- tests/drmsltest.c | 172 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 175 insertions(+), 1 deletion(-) create mode 100644 tests/drmsltest.c
diff --git a/.gitignore b/.gitignore index 06cc928..9c6ecd7 100644 --- a/.gitignore +++ b/.gitignore @@ -75,6 +75,7 @@ via.kld tests/auth tests/dristat tests/drmstat +tests/drmsltest tests/getclient tests/getstats tests/getversion diff --git a/tests/Makefile.am b/tests/Makefile.am index 10f54e3..ca0f3c7 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -59,7 +59,8 @@ TESTS = \ getstats \ setversion \ updatedraw \ - name_from_fd + name_from_fd \ + drmsltest
check_PROGRAMS += $(TESTS)
diff --git a/tests/drmsltest.c b/tests/drmsltest.c new file mode 100644 index 0000000..d193008 --- /dev/null +++ b/tests/drmsltest.c @@ -0,0 +1,172 @@ +/* xf86drmSL.c -- Skip list support + * Created: Mon May 10 09:28:13 1999 by faith@precisioninsight.com + * + * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas. + * All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * PRECISION INSIGHT AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + * + * Authors: Rickard E. (Rik) Faith faith@valinux.com + * + * DESCRIPTION + * + * This file contains a straightforward skip list implementation.n + * + * FUTURE ENHANCEMENTS + * + * REFERENCES + * + * [Pugh90] William Pugh. Skip Lists: A Probabilistic Alternative to + * Balanced Trees. CACM 33(6), June 1990, pp. 668-676. + * + */ + +#include <stdio.h> +#include <stdlib.h> +#include <sys/time.h> + +#include "xf86drm.h" + +static void print(void* list) +{ + unsigned long key; + void *value; + + if (drmSLFirst(list, &key, &value)) { + do { + printf("key = %5lu, value = %p\n", key, value); + } while (drmSLNext(list, &key, &value)); + } +} + +static double do_time(int size, int iter) +{ + void *list; + int i, j; + unsigned long keys[1000000]; + unsigned long previous; + unsigned long key; + void *value; + struct timeval start, stop; + double usec; + void *ranstate; + + list = drmSLCreate(); + ranstate = drmRandomCreate(12345); + + for (i = 0; i < size; i++) { + keys[i] = drmRandom(ranstate); + drmSLInsert(list, keys[i], NULL); + } + + previous = 0; + if (drmSLFirst(list, &key, &value)) { + do { + if (key <= previous) { + printf( "%lu !< %lu\n", previous, key); + } + previous = key; + } while (drmSLNext(list, &key, &value)); + } + + gettimeofday(&start, NULL); + for (j = 0; j < iter; j++) { + for (i = 0; i < size; i++) { + if (drmSLLookup(list, keys[i], &value)) + printf("Error %lu %d\n", keys[i], i); + } + } + gettimeofday(&stop, NULL); + + usec = (double)(stop.tv_sec * 1000000 + stop.tv_usec + - start.tv_sec * 1000000 - start.tv_usec) / (size * iter); + + printf("%0.2f microseconds for list length %d\n", usec, size); + + drmRandomDouble(ranstate); + drmSLDestroy(list); + + return usec; +} + +static void print_neighbors(void *list, unsigned long key) +{ + unsigned long prev_key = 0; + unsigned long next_key = 0; + void *prev_value; + void *next_value; + int retval; + + retval = drmSLLookupNeighbors(list, key, + &prev_key, &prev_value, + &next_key, &next_value); + printf("Neighbors of %5lu: %d %5lu %5lu\n", + key, retval, prev_key, next_key); +} + +int main(void) +{ + void* list; + double usec, usec2, usec3, usec4; + + list = drmSLCreate(); + printf( "list at %p\n", list); + + print(list); + printf("\n==============================\n\n"); + + drmSLInsert(list, 123, NULL); + drmSLInsert(list, 213, NULL); + drmSLInsert(list, 50, NULL); + print(list); + printf("\n==============================\n\n"); + + print_neighbors(list, 0); + print_neighbors(list, 50); + print_neighbors(list, 51); + print_neighbors(list, 123); + print_neighbors(list, 200); + print_neighbors(list, 213); + print_neighbors(list, 256); + printf("\n==============================\n\n"); + + drmSLDelete(list, 50); + print(list); + printf("\n==============================\n\n"); + + drmSLDump(list); + drmSLDestroy(list); + printf("\n==============================\n\n"); + + usec = do_time(100, 10000); + usec2 = do_time(1000, 500); + printf("Table size increased by %0.2f, search time increased by %0.2f\n", + 1000.0/100.0, usec2 / usec); + + usec3 = do_time(10000, 50); + printf("Table size increased by %0.2f, search time increased by %0.2f\n", + 10000.0/100.0, usec3 / usec); + + usec4 = do_time(100000, 4); + printf("Table size increased by %0.2f, search time increased by %0.2f\n", + 100000.0/100.0, usec4 / usec); + + return 0; +}
On 20/03/15 22:10, Jan Vesely wrote:
Considering that a separate commit removes the test from xf86drmSL.c can you add some information in here. "This moves the test from ...."
Signed-off-by: Jan Vesely jan.vesely@rutgers.edu
.gitignore | 1 + tests/Makefile.am | 3 +- tests/drmsltest.c | 172 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 175 insertions(+), 1 deletion(-) create mode 100644 tests/drmsltest.c
diff --git a/.gitignore b/.gitignore index 06cc928..9c6ecd7 100644 --- a/.gitignore +++ b/.gitignore @@ -75,6 +75,7 @@ via.kld tests/auth tests/dristat tests/drmstat +tests/drmsltest
Perhaps we can omit the trailing test -> tests/drmsl ?
tests/getclient tests/getstats tests/getversion diff --git a/tests/Makefile.am b/tests/Makefile.am index 10f54e3..ca0f3c7 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -59,7 +59,8 @@ TESTS = \ getstats \ setversion \ updatedraw \
- name_from_fd
- name_from_fd \
- drmsltest
Can we try to keep this alphabetically sorted ?
I haven't verified that the code movement, although it seems that you've nuked the trailing whitespace :-)
With the above comments addressed:
Acked-by: Emil Velikov emil.l.velikov@gmail.com
-Emil
v2: merge tests creation and xf86drmSL cleanup rename tests/drmsltest -> tests/drmsl move the test out of libudev test block
Signed-off-by: Jan Vesely jan.vesely@rutgers.edu ---
Hi Emil, I know you send your R-b on the earlier version, but I thought the changes were big enough to send v2. I modeled it after you test splitting series.
jan
.gitignore | 1 + tests/Makefile.am | 5 +- tests/drmsl.c | 172 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ xf86drmSL.c | 172 ++---------------------------------------------------- 4 files changed, 183 insertions(+), 167 deletions(-) create mode 100644 tests/drmsl.c
diff --git a/.gitignore b/.gitignore index 06cc928..cb7128d 100644 --- a/.gitignore +++ b/.gitignore @@ -74,6 +74,7 @@ tdfx.kld via.kld tests/auth tests/dristat +tests/drmsl tests/drmstat tests/getclient tests/getstats diff --git a/tests/Makefile.am b/tests/Makefile.am index 10f54e3..ad70314 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -35,6 +35,9 @@ if HAVE_NOUVEAU SUBDIRS += nouveau endif
+TESTS = \ + drmsl + if HAVE_LIBUDEV
check_LTLIBRARIES = libdrmtest.la @@ -52,7 +55,7 @@ XFAIL_TESTS = \ auth \ lock
-TESTS = \ +TESTS += \ openclose \ getversion \ getclient \ diff --git a/tests/drmsl.c b/tests/drmsl.c new file mode 100644 index 0000000..d0ac0ef --- /dev/null +++ b/tests/drmsl.c @@ -0,0 +1,172 @@ +/* drmsl.c -- Skip list test + * Created: Mon May 10 09:28:13 1999 by faith@precisioninsight.com + * + * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas. + * All Rights Reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * PRECISION INSIGHT AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + * + * Authors: Rickard E. (Rik) Faith faith@valinux.com + * + * DESCRIPTION + * + * This file contains a straightforward skip list implementation.n + * + * FUTURE ENHANCEMENTS + * + * REFERENCES + * + * [Pugh90] William Pugh. Skip Lists: A Probabilistic Alternative to + * Balanced Trees. CACM 33(6), June 1990, pp. 668-676. + * + */ + +#include <stdio.h> +#include <stdlib.h> +#include <sys/time.h> + +#include "xf86drm.h" + +static void print(void* list) +{ + unsigned long key; + void *value; + + if (drmSLFirst(list, &key, &value)) { + do { + printf("key = %5lu, value = %p\n", key, value); + } while (drmSLNext(list, &key, &value)); + } +} + +static double do_time(int size, int iter) +{ + void *list; + int i, j; + unsigned long keys[1000000]; + unsigned long previous; + unsigned long key; + void *value; + struct timeval start, stop; + double usec; + void *ranstate; + + list = drmSLCreate(); + ranstate = drmRandomCreate(12345); + + for (i = 0; i < size; i++) { + keys[i] = drmRandom(ranstate); + drmSLInsert(list, keys[i], NULL); + } + + previous = 0; + if (drmSLFirst(list, &key, &value)) { + do { + if (key <= previous) { + printf( "%lu !< %lu\n", previous, key); + } + previous = key; + } while (drmSLNext(list, &key, &value)); + } + + gettimeofday(&start, NULL); + for (j = 0; j < iter; j++) { + for (i = 0; i < size; i++) { + if (drmSLLookup(list, keys[i], &value)) + printf("Error %lu %d\n", keys[i], i); + } + } + gettimeofday(&stop, NULL); + + usec = (double)(stop.tv_sec * 1000000 + stop.tv_usec + - start.tv_sec * 1000000 - start.tv_usec) / (size * iter); + + printf("%0.2f microseconds for list length %d\n", usec, size); + + drmRandomDouble(ranstate); + drmSLDestroy(list); + + return usec; +} + +static void print_neighbors(void *list, unsigned long key) +{ + unsigned long prev_key = 0; + unsigned long next_key = 0; + void *prev_value; + void *next_value; + int retval; + + retval = drmSLLookupNeighbors(list, key, + &prev_key, &prev_value, + &next_key, &next_value); + printf("Neighbors of %5lu: %d %5lu %5lu\n", + key, retval, prev_key, next_key); +} + +int main(void) +{ + void* list; + double usec, usec2, usec3, usec4; + + list = drmSLCreate(); + printf( "list at %p\n", list); + + print(list); + printf("\n==============================\n\n"); + + drmSLInsert(list, 123, NULL); + drmSLInsert(list, 213, NULL); + drmSLInsert(list, 50, NULL); + print(list); + printf("\n==============================\n\n"); + + print_neighbors(list, 0); + print_neighbors(list, 50); + print_neighbors(list, 51); + print_neighbors(list, 123); + print_neighbors(list, 200); + print_neighbors(list, 213); + print_neighbors(list, 256); + printf("\n==============================\n\n"); + + drmSLDelete(list, 50); + print(list); + printf("\n==============================\n\n"); + + drmSLDump(list); + drmSLDestroy(list); + printf("\n==============================\n\n"); + + usec = do_time(100, 10000); + usec2 = do_time(1000, 500); + printf("Table size increased by %0.2f, search time increased by %0.2f\n", + 1000.0/100.0, usec2 / usec); + + usec3 = do_time(10000, 50); + printf("Table size increased by %0.2f, search time increased by %0.2f\n", + 10000.0/100.0, usec3 / usec); + + usec4 = do_time(100000, 4); + printf("Table size increased by %0.2f, search time increased by %0.2f\n", + 100000.0/100.0, usec4 / usec); + + return 0; +} diff --git a/xf86drmSL.c b/xf86drmSL.c index cf588ac..bb9ca7f 100644 --- a/xf86drmSL.c +++ b/xf86drmSL.c @@ -41,13 +41,7 @@ #include <stdio.h> #include <stdlib.h>
-#define SL_MAIN 0 - -#if !SL_MAIN -# include "xf86drm.h" -#else -# include <sys/time.h> -#endif +#include "xf86drm.h"
#define SL_LIST_MAGIC 0xfacade00LU #define SL_ENTRY_MAGIC 0x00fab1edLU @@ -56,21 +50,10 @@ #define SL_DEBUG 0 #define SL_RANDOM_SEED 0xc01055a1LU
-#if SL_MAIN -#define SL_ALLOC malloc -#define SL_FREE free -#define SL_RANDOM_DECL static int state = 0; -#define SL_RANDOM_INIT(seed) if (!state) { srandom(seed); ++state; } -#define SL_RANDOM random() -#else -#define SL_ALLOC drmMalloc -#define SL_FREE drmFree #define SL_RANDOM_DECL static void *state = NULL #define SL_RANDOM_INIT(seed) if (!state) state = drmRandomCreate(seed) #define SL_RANDOM drmRandom(state)
-#endif - typedef struct SLEntry { unsigned long magic; /* SL_ENTRY_MAGIC */ unsigned long key; @@ -87,27 +70,13 @@ typedef struct SkipList { SLEntryPtr p0; /* Position for iteration */ } SkipList, *SkipListPtr;
-#if SL_MAIN -extern void *drmSLCreate(void); -extern int drmSLDestroy(void *l); -extern int drmSLLookup(void *l, unsigned long key, void **value); -extern int drmSLInsert(void *l, unsigned long key, void *value); -extern int drmSLDelete(void *l, unsigned long key); -extern int drmSLNext(void *l, unsigned long *key, void **value); -extern int drmSLFirst(void *l, unsigned long *key, void **value); -extern void drmSLDump(void *l); -extern int drmSLLookupNeighbors(void *l, unsigned long key, - unsigned long *prev_key, void **prev_value, - unsigned long *next_key, void **next_value); -#endif - static SLEntryPtr SLCreateEntry(int max_level, unsigned long key, void *value) { SLEntryPtr entry;
if (max_level < 0 || max_level > SL_MAX_LEVEL) max_level = SL_MAX_LEVEL;
- entry = SL_ALLOC(sizeof(*entry) + entry = drmMalloc(sizeof(*entry) + (max_level + 1) * sizeof(entry->forward[0])); if (!entry) return NULL; entry->magic = SL_ENTRY_MAGIC; @@ -134,7 +103,7 @@ void *drmSLCreate(void) SkipListPtr list; int i;
- list = SL_ALLOC(sizeof(*list)); + list = drmMalloc(sizeof(*list)); if (!list) return NULL; list->magic = SL_LIST_MAGIC; list->level = 0; @@ -158,11 +127,11 @@ int drmSLDestroy(void *l) if (entry->magic != SL_ENTRY_MAGIC) return -1; /* Bad magic */ next = entry->forward[0]; entry->magic = SL_FREED_MAGIC; - SL_FREE(entry); + drmFree(entry); }
list->magic = SL_FREED_MAGIC; - SL_FREE(list); + drmFree(list); return 0; }
@@ -236,7 +205,7 @@ int drmSLDelete(void *l, unsigned long key) }
entry->magic = SL_FREED_MAGIC; - SL_FREE(entry); + drmFree(entry);
while (list->level && !list->head->forward[list->level]) --list->level; --list->count; @@ -348,132 +317,3 @@ void drmSLDump(void *l) } } } - -#if SL_MAIN -static void print(SkipListPtr list) -{ - unsigned long key; - void *value; - - if (drmSLFirst(list, &key, &value)) { - do { - printf("key = %5lu, value = %p\n", key, value); - } while (drmSLNext(list, &key, &value)); - } -} - -static double do_time(int size, int iter) -{ - SkipListPtr list; - int i, j; - unsigned long keys[1000000]; - unsigned long previous; - unsigned long key; - void *value; - struct timeval start, stop; - double usec; - SL_RANDOM_DECL; - - SL_RANDOM_INIT(12345); - - list = drmSLCreate(); - - for (i = 0; i < size; i++) { - keys[i] = SL_RANDOM; - drmSLInsert(list, keys[i], NULL); - } - - previous = 0; - if (drmSLFirst(list, &key, &value)) { - do { - if (key <= previous) { - printf( "%lu !< %lu\n", previous, key); - } - previous = key; - } while (drmSLNext(list, &key, &value)); - } - - gettimeofday(&start, NULL); - for (j = 0; j < iter; j++) { - for (i = 0; i < size; i++) { - if (drmSLLookup(list, keys[i], &value)) - printf("Error %lu %d\n", keys[i], i); - } - } - gettimeofday(&stop, NULL); - - usec = (double)(stop.tv_sec * 1000000 + stop.tv_usec - - start.tv_sec * 1000000 - start.tv_usec) / (size * iter); - - printf("%0.2f microseconds for list length %d\n", usec, size); - - drmSLDestroy(list); - - return usec; -} - -static void print_neighbors(void *list, unsigned long key) -{ - unsigned long prev_key = 0; - unsigned long next_key = 0; - void *prev_value; - void *next_value; - int retval; - - retval = drmSLLookupNeighbors(list, key, - &prev_key, &prev_value, - &next_key, &next_value); - printf("Neighbors of %5lu: %d %5lu %5lu\n", - key, retval, prev_key, next_key); -} - -int main(void) -{ - SkipListPtr list; - double usec, usec2, usec3, usec4; - - list = drmSLCreate(); - printf( "list at %p\n", list); - - print(list); - printf("\n==============================\n\n"); - - drmSLInsert(list, 123, NULL); - drmSLInsert(list, 213, NULL); - drmSLInsert(list, 50, NULL); - print(list); - printf("\n==============================\n\n"); - - print_neighbors(list, 0); - print_neighbors(list, 50); - print_neighbors(list, 51); - print_neighbors(list, 123); - print_neighbors(list, 200); - print_neighbors(list, 213); - print_neighbors(list, 256); - printf("\n==============================\n\n"); - - drmSLDelete(list, 50); - print(list); - printf("\n==============================\n\n"); - - drmSLDump(list); - drmSLDestroy(list); - printf("\n==============================\n\n"); - - usec = do_time(100, 10000); - usec2 = do_time(1000, 500); - printf("Table size increased by %0.2f, search time increased by %0.2f\n", - 1000.0/100.0, usec2 / usec); - - usec3 = do_time(10000, 50); - printf("Table size increased by %0.2f, search time increased by %0.2f\n", - 10000.0/100.0, usec3 / usec); - - usec4 = do_time(100000, 4); - printf("Table size increased by %0.2f, search time increased by %0.2f\n", - 100000.0/100.0, usec4 / usec); - - return 0; -} -#endif
On 24/03/15 23:06, Jan Vesely wrote:
v2: merge tests creation and xf86drmSL cleanup rename tests/drmsltest -> tests/drmsl move the test out of libudev test block
Signed-off-by: Jan Vesely jan.vesely@rutgers.edu
Hi Emil, I know you send your R-b on the earlier version, but I thought the changes were big enough to send v2. I modeled it after you test splitting series.
jan
.gitignore | 1 + tests/Makefile.am | 5 +- tests/drmsl.c | 172 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ xf86drmSL.c | 172 ++---------------------------------------------------- 4 files changed, 183 insertions(+), 167 deletions(-) create mode 100644 tests/drmsl.c
diff --git a/.gitignore b/.gitignore index 06cc928..cb7128d 100644 --- a/.gitignore +++ b/.gitignore @@ -74,6 +74,7 @@ tdfx.kld via.kld tests/auth tests/dristat +tests/drmsl tests/drmstat tests/getclient tests/getstats
Hmm this reminds me that I missed out updating the .gitignore with my series.
diff --git a/tests/Makefile.am b/tests/Makefile.am index 10f54e3..ad70314 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -35,6 +35,9 @@ if HAVE_NOUVEAU SUBDIRS += nouveau endif
+TESTS = \
- drmsl
if HAVE_LIBUDEV
check_LTLIBRARIES = libdrmtest.la @@ -52,7 +55,7 @@ XFAIL_TESTS = \ auth \ lock
-TESTS = \ +TESTS += \ openclose \ getversion \ getclient \
You will need the following hunk, otherwise drmsl will not end up in the check target. Although when I think about it there isn't much value in running this at make check time. I'll leave the decision up-to you.
--- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -61,6 +61,7 @@ TESTS = \ updatedraw \ name_from_fd
+endif + check_PROGRAMS += $(TESTS)
-endif
As before haven't explicitly checked the moved code so
Acked-by: Emil Velikov emil.l.velikov@gmail.com
Thanks Emil
Signed-off-by: Jan Vesely jan.vesely@rutgers.edu --- tests/drmsltest.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/tests/drmsltest.c b/tests/drmsltest.c index d193008..d871fbf 100644 --- a/tests/drmsltest.c +++ b/tests/drmsltest.c @@ -106,7 +106,9 @@ static double do_time(int size, int iter) return usec; }
-static void print_neighbors(void *list, unsigned long key) +static void print_neighbors(void *list, unsigned long key, + unsigned long expected_prev, + unsigned long expected_next) { unsigned long prev_key = 0; unsigned long next_key = 0; @@ -119,6 +121,16 @@ static void print_neighbors(void *list, unsigned long key) &next_key, &next_value); printf("Neighbors of %5lu: %d %5lu %5lu\n", key, retval, prev_key, next_key); + if (prev_key != expected_prev) { + fprintf(stderr, "Unexpected neighbor: %5lu. Expected: %5lu\n", + prev_key, expected_prev); + exit(1); + } + if (next_key != expected_next) { + fprintf(stderr, "Unexpected neighbor: %5lu. Expected: %5lu\n", + next_key, expected_next); + exit(1); + } }
int main(void) @@ -138,13 +150,13 @@ int main(void) print(list); printf("\n==============================\n\n");
- print_neighbors(list, 0); - print_neighbors(list, 50); - print_neighbors(list, 51); - print_neighbors(list, 123); - print_neighbors(list, 200); - print_neighbors(list, 213); - print_neighbors(list, 256); + print_neighbors(list, 0, 0, 50); + print_neighbors(list, 50, 0, 50); + print_neighbors(list, 51, 50, 123); + print_neighbors(list, 123, 50, 123); + print_neighbors(list, 200, 123, 213); + print_neighbors(list, 213, 123, 213); + print_neighbors(list, 256, 213, 256); printf("\n==============================\n\n");
drmSLDelete(list, 50);
Signed-off-by: Jan Vesely jan.vesely@rutgers.edu --- xf86drmSL.c | 172 +++--------------------------------------------------------- 1 file changed, 6 insertions(+), 166 deletions(-)
diff --git a/xf86drmSL.c b/xf86drmSL.c index cf588ac..bb9ca7f 100644 --- a/xf86drmSL.c +++ b/xf86drmSL.c @@ -41,13 +41,7 @@ #include <stdio.h> #include <stdlib.h>
-#define SL_MAIN 0 - -#if !SL_MAIN -# include "xf86drm.h" -#else -# include <sys/time.h> -#endif +#include "xf86drm.h"
#define SL_LIST_MAGIC 0xfacade00LU #define SL_ENTRY_MAGIC 0x00fab1edLU @@ -56,21 +50,10 @@ #define SL_DEBUG 0 #define SL_RANDOM_SEED 0xc01055a1LU
-#if SL_MAIN -#define SL_ALLOC malloc -#define SL_FREE free -#define SL_RANDOM_DECL static int state = 0; -#define SL_RANDOM_INIT(seed) if (!state) { srandom(seed); ++state; } -#define SL_RANDOM random() -#else -#define SL_ALLOC drmMalloc -#define SL_FREE drmFree #define SL_RANDOM_DECL static void *state = NULL #define SL_RANDOM_INIT(seed) if (!state) state = drmRandomCreate(seed) #define SL_RANDOM drmRandom(state)
-#endif - typedef struct SLEntry { unsigned long magic; /* SL_ENTRY_MAGIC */ unsigned long key; @@ -87,27 +70,13 @@ typedef struct SkipList { SLEntryPtr p0; /* Position for iteration */ } SkipList, *SkipListPtr;
-#if SL_MAIN -extern void *drmSLCreate(void); -extern int drmSLDestroy(void *l); -extern int drmSLLookup(void *l, unsigned long key, void **value); -extern int drmSLInsert(void *l, unsigned long key, void *value); -extern int drmSLDelete(void *l, unsigned long key); -extern int drmSLNext(void *l, unsigned long *key, void **value); -extern int drmSLFirst(void *l, unsigned long *key, void **value); -extern void drmSLDump(void *l); -extern int drmSLLookupNeighbors(void *l, unsigned long key, - unsigned long *prev_key, void **prev_value, - unsigned long *next_key, void **next_value); -#endif - static SLEntryPtr SLCreateEntry(int max_level, unsigned long key, void *value) { SLEntryPtr entry;
if (max_level < 0 || max_level > SL_MAX_LEVEL) max_level = SL_MAX_LEVEL;
- entry = SL_ALLOC(sizeof(*entry) + entry = drmMalloc(sizeof(*entry) + (max_level + 1) * sizeof(entry->forward[0])); if (!entry) return NULL; entry->magic = SL_ENTRY_MAGIC; @@ -134,7 +103,7 @@ void *drmSLCreate(void) SkipListPtr list; int i;
- list = SL_ALLOC(sizeof(*list)); + list = drmMalloc(sizeof(*list)); if (!list) return NULL; list->magic = SL_LIST_MAGIC; list->level = 0; @@ -158,11 +127,11 @@ int drmSLDestroy(void *l) if (entry->magic != SL_ENTRY_MAGIC) return -1; /* Bad magic */ next = entry->forward[0]; entry->magic = SL_FREED_MAGIC; - SL_FREE(entry); + drmFree(entry); }
list->magic = SL_FREED_MAGIC; - SL_FREE(list); + drmFree(list); return 0; }
@@ -236,7 +205,7 @@ int drmSLDelete(void *l, unsigned long key) }
entry->magic = SL_FREED_MAGIC; - SL_FREE(entry); + drmFree(entry);
while (list->level && !list->head->forward[list->level]) --list->level; --list->count; @@ -348,132 +317,3 @@ void drmSLDump(void *l) } } } - -#if SL_MAIN -static void print(SkipListPtr list) -{ - unsigned long key; - void *value; - - if (drmSLFirst(list, &key, &value)) { - do { - printf("key = %5lu, value = %p\n", key, value); - } while (drmSLNext(list, &key, &value)); - } -} - -static double do_time(int size, int iter) -{ - SkipListPtr list; - int i, j; - unsigned long keys[1000000]; - unsigned long previous; - unsigned long key; - void *value; - struct timeval start, stop; - double usec; - SL_RANDOM_DECL; - - SL_RANDOM_INIT(12345); - - list = drmSLCreate(); - - for (i = 0; i < size; i++) { - keys[i] = SL_RANDOM; - drmSLInsert(list, keys[i], NULL); - } - - previous = 0; - if (drmSLFirst(list, &key, &value)) { - do { - if (key <= previous) { - printf( "%lu !< %lu\n", previous, key); - } - previous = key; - } while (drmSLNext(list, &key, &value)); - } - - gettimeofday(&start, NULL); - for (j = 0; j < iter; j++) { - for (i = 0; i < size; i++) { - if (drmSLLookup(list, keys[i], &value)) - printf("Error %lu %d\n", keys[i], i); - } - } - gettimeofday(&stop, NULL); - - usec = (double)(stop.tv_sec * 1000000 + stop.tv_usec - - start.tv_sec * 1000000 - start.tv_usec) / (size * iter); - - printf("%0.2f microseconds for list length %d\n", usec, size); - - drmSLDestroy(list); - - return usec; -} - -static void print_neighbors(void *list, unsigned long key) -{ - unsigned long prev_key = 0; - unsigned long next_key = 0; - void *prev_value; - void *next_value; - int retval; - - retval = drmSLLookupNeighbors(list, key, - &prev_key, &prev_value, - &next_key, &next_value); - printf("Neighbors of %5lu: %d %5lu %5lu\n", - key, retval, prev_key, next_key); -} - -int main(void) -{ - SkipListPtr list; - double usec, usec2, usec3, usec4; - - list = drmSLCreate(); - printf( "list at %p\n", list); - - print(list); - printf("\n==============================\n\n"); - - drmSLInsert(list, 123, NULL); - drmSLInsert(list, 213, NULL); - drmSLInsert(list, 50, NULL); - print(list); - printf("\n==============================\n\n"); - - print_neighbors(list, 0); - print_neighbors(list, 50); - print_neighbors(list, 51); - print_neighbors(list, 123); - print_neighbors(list, 200); - print_neighbors(list, 213); - print_neighbors(list, 256); - printf("\n==============================\n\n"); - - drmSLDelete(list, 50); - print(list); - printf("\n==============================\n\n"); - - drmSLDump(list); - drmSLDestroy(list); - printf("\n==============================\n\n"); - - usec = do_time(100, 10000); - usec2 = do_time(1000, 500); - printf("Table size increased by %0.2f, search time increased by %0.2f\n", - 1000.0/100.0, usec2 / usec); - - usec3 = do_time(10000, 50); - printf("Table size increased by %0.2f, search time increased by %0.2f\n", - 10000.0/100.0, usec3 / usec); - - usec4 = do_time(100000, 4); - printf("Table size increased by %0.2f, search time increased by %0.2f\n", - 100000.0/100.0, usec4 / usec); - - return 0; -} -#endif
On 20/03/15 22:10, Jan Vesely wrote:
Signed-off-by: Jan Vesely jan.vesely@rutgers.edu
Can you mention in the commit message that the test was moved to tests/drmsl ?
Reviewed-by: Emil Velikov emil.l.velikov@gmail.com
I'm not planning to look at patch 3/4.
Thanks for the split, I'll shortly follow up with the other two plus a trivial cleanup :-)
-Emil
On 20/03/15 22:10, Jan Vesely wrote:
v2: zero the update array instead of checking the return value. SLLocate returns NULL both on failure and if the element is greater than everything in the list
Can we add to the commit message what the issue is and mention the commit which caused this regression ? With that
Acked-by: Emil Velikov emil.l.velikov@gmail.com
-Emil
Commit e4a519635f75bde38aeb5b09f2ff4efbf73453e9: Tidy up compile warnings by cleaning up types.
removed call to SLLocate which gutted the function of all functionality. This patch restores the original behavior, with an additional fix that zeros the update array in case SLLocate bails early.
v2: zero the update array instead of checking the return value. SLLocate returns NULL both on failure and if the element is greater than everything in the list v3: Improve commit message
Signed-off-by: Jan Vesely jan.vesely@rutgers.edu Acked-by: Emil Velikov emil.l.velikov@gmail.com --- sorry for spamming, just realized it might be a good idea co CC the original author and committer.
This was broken since 2.4.18 (2010). I guess it's safe to say that nobody uses it. What are the policies on removing parts of API?
xf86drmSL.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/xf86drmSL.c b/xf86drmSL.c index acddb54..cf588ac 100644 --- a/xf86drmSL.c +++ b/xf86drmSL.c @@ -264,12 +264,14 @@ int drmSLLookupNeighbors(void *l, unsigned long key, unsigned long *next_key, void **next_value) { SkipListPtr list = (SkipListPtr)l; - SLEntryPtr update[SL_MAX_LEVEL + 1]; + SLEntryPtr update[SL_MAX_LEVEL + 1] = {0}; int retcode = 0;
+ SLLocate(list, key, update); + *prev_key = *next_key = key; *prev_value = *next_value = NULL; - + if (update[0]) { *prev_key = update[0]->key; *prev_value = update[0]->value;
On 24/03/15 23:16, Jan Vesely wrote:
Commit e4a519635f75bde38aeb5b09f2ff4efbf73453e9: Tidy up compile warnings by cleaning up types.
removed call to SLLocate which gutted the function of all functionality. This patch restores the original behavior, with an additional fix that zeros the update array in case SLLocate bails early.
v2: zero the update array instead of checking the return value. SLLocate returns NULL both on failure and if the element is greater than everything in the list v3: Improve commit message
Signed-off-by: Jan Vesely jan.vesely@rutgers.edu Acked-by: Emil Velikov emil.l.velikov@gmail.com
sorry for spamming, just realized it might be a good idea co CC the original author and committer.
This was broken since 2.4.18 (2010). I guess it's safe to say that nobody uses it. What are the policies on removing parts of API?
Either nobody uses it or nobody which uses it cases enough to report/fix it. Either way I think that people prefer to be cautious and do API cleanups alongside a major bump of the library.
-Emil
dri-devel@lists.freedesktop.org
-
Emil Velikov -
Jan Vesely