FT-258 Remove a malloc from the cursor creation path

ft/cursor.cc

@@ -91,61 +91,73 @@ PATENT RIGHTS GRANT:
 #include "ft/cursor.h"
 #include "ft/leafentry.h"
 #include "ft/txn.h"
+#include "ft/ybt.h"
 
-int toku_ft_cursor(FT_HANDLE ft_handle, FT_CURSOR *cursorptr, TOKUTXN ttxn,
+int toku_ft_cursor_create(FT_HANDLE ft_handle, FT_CURSOR cursor, TOKUTXN ttxn,
                           bool is_snapshot_read, bool disable_prefetching) {
     if (is_snapshot_read) {
         invariant(ttxn != NULL);
         int accepted = toku_txn_reads_txnid(ft_handle->ft->h->root_xid_that_created, ttxn);
-        if (accepted!=TOKUDB_ACCEPT) {
-            invariant(accepted==0);
+        if (accepted != TOKUDB_ACCEPT) {
+            invariant(accepted == 0);
             return TOKUDB_MVCC_DICTIONARY_TOO_NEW;
         }
     }
-    FT_CURSOR XCALLOC(cursor);
+
+    memset(cursor, 0, sizeof(*cursor));
     cursor->ft_handle = ft_handle;
-    cursor->prefetching = false;
-    toku_init_dbt(&cursor->range_lock_left_key);
-    toku_init_dbt(&cursor->range_lock_right_key);
-    cursor->left_is_neg_infty = false;
-    cursor->right_is_pos_infty = false;
     cursor->is_snapshot_read = is_snapshot_read;
-    cursor->is_leaf_mode = false;
     cursor->ttxn = ttxn;
     cursor->disable_prefetching = disable_prefetching;
-    cursor->is_temporary = false;
-    *cursorptr = cursor;
     return 0;
 }
 
-void toku_ft_cursor_close(FT_CURSOR cursor) {
+void toku_ft_cursor_destroy(FT_CURSOR cursor) {
     toku_destroy_dbt(&cursor->key);
     toku_destroy_dbt(&cursor->val);
     toku_destroy_dbt(&cursor->range_lock_left_key);
     toku_destroy_dbt(&cursor->range_lock_right_key);
+}
+
+// deprecated, should only be used by tests
+int toku_ft_cursor(FT_HANDLE ft_handle, FT_CURSOR *cursorptr, TOKUTXN ttxn,
+                   bool is_snapshot_read, bool disable_prefetching) {
+    FT_CURSOR XCALLOC(cursor);
+    int r = toku_ft_cursor_create(ft_handle, cursor, ttxn, is_snapshot_read, disable_prefetching);
+    if (r == 0) {
+        *cursorptr = cursor;
+    } else {
+        toku_free(cursor);
+    }
+    return r;
+}
+
+// deprecated, should only be used by tests
+void toku_ft_cursor_close(FT_CURSOR cursor) {
+    toku_ft_cursor_destroy(cursor);
     toku_free(cursor);
 }
 
-void toku_ft_cursor_remove_restriction(FT_CURSOR ftcursor) {
-    ftcursor->out_of_range_error = 0;
-    ftcursor->direction = 0;
+void toku_ft_cursor_remove_restriction(FT_CURSOR cursor) {
+    cursor->out_of_range_error = 0;
+    cursor->direction = 0;
 }
 
-void toku_ft_cursor_set_check_interrupt_cb(FT_CURSOR ftcursor, FT_CHECK_INTERRUPT_CALLBACK cb, void *extra) {
-    ftcursor->interrupt_cb = cb;
-    ftcursor->interrupt_cb_extra = extra;
+void toku_ft_cursor_set_check_interrupt_cb(FT_CURSOR cursor, FT_CHECK_INTERRUPT_CALLBACK cb, void *extra) {
+    cursor->interrupt_cb = cb;
+    cursor->interrupt_cb_extra = extra;
 }
 
-void toku_ft_cursor_set_temporary(FT_CURSOR ftcursor) {
-    ftcursor->is_temporary = true;
+void toku_ft_cursor_set_temporary(FT_CURSOR cursor) {
+    cursor->is_temporary = true;
 }
 
-void toku_ft_cursor_set_leaf_mode(FT_CURSOR ftcursor) {
-    ftcursor->is_leaf_mode = true;
+void toku_ft_cursor_set_leaf_mode(FT_CURSOR cursor) {
+    cursor->is_leaf_mode = true;
 }
 
-int toku_ft_cursor_is_leaf_mode(FT_CURSOR ftcursor) {
-    return ftcursor->is_leaf_mode;
+int toku_ft_cursor_is_leaf_mode(FT_CURSOR cursor) {
+    return cursor->is_leaf_mode;
 }
 
 // TODO: Rename / cleanup - this has nothing to do with locking

ft/cursor.h

@@ -174,9 +174,12 @@ static inline void ft_search_finish(ft_search *search) {
     toku_destroy_dbt(&search->pivot_bound);
 }
 
-int toku_ft_lookup (FT_HANDLE ft_h, DBT *k, FT_GET_CALLBACK_FUNCTION getf, void *getf_v)  __attribute__ ((warn_unused_result));
 
-int toku_ft_cursor(FT_HANDLE ft_handle, FT_CURSOR*, TOKUTXN, bool, bool) __attribute__ ((warn_unused_result));
+int toku_ft_cursor_create(FT_HANDLE ft_handle, FT_CURSOR cursor, TOKUTXN txn, bool, bool);
+
+void toku_ft_cursor_destroy(FT_CURSOR cursor);
+
+int toku_ft_lookup(FT_HANDLE ft_h, DBT *k, FT_GET_CALLBACK_FUNCTION getf, void *getf_v)  __attribute__ ((warn_unused_result));
 
 void toku_ft_cursor_set_prefetching(FT_CURSOR cursor);
 
@@ -213,8 +216,6 @@ int toku_ft_cursor_set_range(FT_CURSOR cursor, DBT *key, DBT *key_bound, FT_GET_
 
 int toku_ft_cursor_set_range_reverse(FT_CURSOR cursor, DBT *key, FT_GET_CALLBACK_FUNCTION getf, void *getf_v) __attribute__ ((warn_unused_result));
 
-void toku_ft_cursor_close(FT_CURSOR cursor);
-
 bool toku_ft_cursor_uninitialized(FT_CURSOR cursor) __attribute__ ((warn_unused_result));
 
 void toku_ft_cursor_peek(FT_CURSOR cursor, const DBT **pkey, const DBT **pval);
@@ -225,10 +226,12 @@ int toku_ft_cursor_shortcut(FT_CURSOR cursor, int direction, uint32_t index, bn_
                             FT_GET_CALLBACK_FUNCTION getf, void *getf_v,
                             uint32_t *keylen, void **key, uint32_t *vallen, void **val);
 
-// deprecated
-int toku_ft_cursor_get(FT_CURSOR cursor, DBT *key, FT_GET_CALLBACK_FUNCTION getf, void *getf_v, int get_flags);
-int toku_ft_cursor_delete(FT_CURSOR cursor, int flags, TOKUTXN txn);
-
 // used by get_key_after_bytes
 int toku_ft_cursor_compare_one(const ft_search &search, const DBT *x);
 int toku_ft_cursor_compare_set_range(const ft_search &search, const DBT *x);
+
+// deprecated, should only be used by tests, and eventually removed
+int toku_ft_cursor(FT_HANDLE ft_handle, FT_CURSOR *ftcursor_p, TOKUTXN txn, bool, bool) __attribute__ ((warn_unused_result));
+void toku_ft_cursor_close(FT_CURSOR cursor);
+int toku_ft_cursor_get(FT_CURSOR cursor, DBT *key, FT_GET_CALLBACK_FUNCTION getf, void *getf_v, int get_flags);
+int toku_ft_cursor_delete(FT_CURSOR cursor, int flags, TOKUTXN txn);

src/ydb-internal.h

@@ -95,6 +95,7 @@ PATENT RIGHTS GRANT:
 #include <limits.h>
 
 #include <ft/cachetable.h>
+#include <ft/cursor.h>
 #include <ft/fttypes.h>
 #include <ft/logger.h>
 #include <ft/txn.h>
@@ -278,7 +279,7 @@ struct __toku_db_txn_external {
 #define db_txn_struct_i(x) (&((struct __toku_db_txn_external *)x)->internal_part)
 
 struct __toku_dbc_internal {
-    struct ft_cursor *c;
+    struct ft_cursor ftcursor;
     DB_TXN *txn;
     TOKU_ISOLATION iso;
     struct simple_dbt skey_s,sval_s;
@@ -296,6 +297,10 @@ struct __toku_dbc_external {
 	
 #define dbc_struct_i(x) (&((struct __toku_dbc_external *)x)->internal_part)
 
+static inline struct ft_cursor *dbc_ftcursor(DBC *c) {
+    return &dbc_struct_i(c)->ftcursor;
+}
+
 static inline int 
 env_opened(DB_ENV *env) {
     return env->i->cachetable != 0;

src/ydb_cursor.cc

@@ -134,8 +134,8 @@ get_nonmain_cursor_flags(uint32_t flags) {
 }
 
 static inline bool 
-c_uninitialized(DBC* c) {
-    return toku_ft_cursor_uninitialized(dbc_struct_i(c)->c);
+c_uninitialized(DBC *c) {
+    return toku_ft_cursor_uninitialized(dbc_ftcursor(c));
 }            
 
 typedef struct query_context_wrapped_t {
@@ -201,7 +201,7 @@ typedef struct query_context_with_input_t {
 
 static void
 query_context_base_init(QUERY_CONTEXT_BASE context, DBC *c, uint32_t flag, bool is_write_op, YDB_CALLBACK_FUNCTION f, void *extra) {
-    context->c       = dbc_struct_i(c)->c;
+    context->c       = dbc_ftcursor(c);
     context->txn     = dbc_struct_i(c)->txn;
     context->db      = c->dbp;
     context->f       = f;
@@ -278,7 +278,7 @@ c_getf_first(DBC *c, uint32_t flag, YDB_CALLBACK_FUNCTION f, void *extra) {
     c_query_context_init(&context, c, flag, f, extra);
     while (r == 0) {
         //toku_ft_cursor_first will call c_getf_first_callback(..., context) (if query is successful)
-        r = toku_ft_cursor_first(dbc_struct_i(c)->c, c_getf_first_callback, &context);
+        r = toku_ft_cursor_first(dbc_ftcursor(c), c_getf_first_callback, &context);
         if (r == DB_LOCK_NOTGRANTED) {
             r = toku_db_wait_range_lock(context.base.db, context.base.txn, &context.base.request);
         } else {
@@ -329,7 +329,7 @@ c_getf_last(DBC *c, uint32_t flag, YDB_CALLBACK_FUNCTION f, void *extra) {
     c_query_context_init(&context, c, flag, f, extra); 
     while (r == 0) {
         //toku_ft_cursor_last will call c_getf_last_callback(..., context) (if query is successful)
-        r = toku_ft_cursor_last(dbc_struct_i(c)->c, c_getf_last_callback, &context);
+        r = toku_ft_cursor_last(dbc_ftcursor(c), c_getf_last_callback, &context);
         if (r == DB_LOCK_NOTGRANTED) {
             r = toku_db_wait_range_lock(context.base.db, context.base.txn, &context.base.request);
         } else {
@@ -384,7 +384,7 @@ c_getf_next(DBC *c, uint32_t flag, YDB_CALLBACK_FUNCTION f, void *extra) {
         c_query_context_init(&context, c, flag, f, extra); 
         while (r == 0) {
             //toku_ft_cursor_next will call c_getf_next_callback(..., context) (if query is successful)
-            r = toku_ft_cursor_next(dbc_struct_i(c)->c, c_getf_next_callback, &context);
+            r = toku_ft_cursor_next(dbc_ftcursor(c), c_getf_next_callback, &context);
             if (r == DB_LOCK_NOTGRANTED) {
                 r = toku_db_wait_range_lock(context.base.db, context.base.txn, &context.base.request);
             } else {
@@ -443,7 +443,7 @@ c_getf_prev(DBC *c, uint32_t flag, YDB_CALLBACK_FUNCTION f, void *extra) {
         c_query_context_init(&context, c, flag, f, extra);
         while (r == 0) {
             //toku_ft_cursor_prev will call c_getf_prev_callback(..., context) (if query is successful)
-            r = toku_ft_cursor_prev(dbc_struct_i(c)->c, c_getf_prev_callback, &context);
+            r = toku_ft_cursor_prev(dbc_ftcursor(c), c_getf_prev_callback, &context);
             if (r == DB_LOCK_NOTGRANTED) {
                 r = toku_db_wait_range_lock(context.base.db, context.base.txn, &context.base.request);
             } else {
@@ -496,7 +496,7 @@ c_getf_current(DBC *c, uint32_t flag, YDB_CALLBACK_FUNCTION f, void *extra) {
     QUERY_CONTEXT_S context; //Describes the context of this query.
     c_query_context_init(&context, c, flag, f, extra); 
     //toku_ft_cursor_current will call c_getf_current_callback(..., context) (if query is successful)
-    int r = toku_ft_cursor_current(dbc_struct_i(c)->c, DB_CURRENT, c_getf_current_callback, &context);
+    int r = toku_ft_cursor_current(dbc_ftcursor(c), DB_CURRENT, c_getf_current_callback, &context);
     c_query_context_destroy(&context);
     return r;
 }
@@ -535,7 +535,7 @@ toku_c_getf_set(DBC *c, uint32_t flag, DBT *key, YDB_CALLBACK_FUNCTION f, void *
     query_context_with_input_init(&context, c, flag, key, NULL, f, extra); 
     while (r == 0) {
         //toku_ft_cursor_set will call c_getf_set_callback(..., context) (if query is successful)
-        r = toku_ft_cursor_set(dbc_struct_i(c)->c, key, c_getf_set_callback, &context);
+        r = toku_ft_cursor_set(dbc_ftcursor(c), key, c_getf_set_callback, &context);
         if (r == DB_LOCK_NOTGRANTED) {
             r = toku_db_wait_range_lock(context.base.db, context.base.txn, &context.base.request);
         } else {
@@ -588,7 +588,7 @@ c_getf_set_range(DBC *c, uint32_t flag, DBT *key, YDB_CALLBACK_FUNCTION f, void
     query_context_with_input_init(&context, c, flag, key, NULL, f, extra); 
     while (r == 0) {
         //toku_ft_cursor_set_range will call c_getf_set_range_callback(..., context) (if query is successful)
-        r = toku_ft_cursor_set_range(dbc_struct_i(c)->c, key, nullptr, c_getf_set_range_callback, &context);
+        r = toku_ft_cursor_set_range(dbc_ftcursor(c), key, nullptr, c_getf_set_range_callback, &context);
         if (r == DB_LOCK_NOTGRANTED) {
             r = toku_db_wait_range_lock(context.base.db, context.base.txn, &context.base.request);
         } else {
@@ -642,7 +642,7 @@ c_getf_set_range_with_bound(DBC *c, uint32_t flag, DBT *key, DBT *key_bound, YDB
     query_context_with_input_init(&context, c, flag, key, NULL, f, extra); 
     while (r == 0) {
         //toku_ft_cursor_set_range will call c_getf_set_range_callback(..., context) (if query is successful)
-        r = toku_ft_cursor_set_range(dbc_struct_i(c)->c, key, key_bound, c_getf_set_range_callback, &context);
+        r = toku_ft_cursor_set_range(dbc_ftcursor(c), key, key_bound, c_getf_set_range_callback, &context);
         if (r == DB_LOCK_NOTGRANTED) {
             r = toku_db_wait_range_lock(context.base.db, context.base.txn, &context.base.request);
         } else {
@@ -665,7 +665,7 @@ c_getf_set_range_reverse(DBC *c, uint32_t flag, DBT *key, YDB_CALLBACK_FUNCTION
     query_context_with_input_init(&context, c, flag, key, NULL, f, extra); 
     while (r == 0) {
         //toku_ft_cursor_set_range_reverse will call c_getf_set_range_reverse_callback(..., context) (if query is successful)
-        r = toku_ft_cursor_set_range_reverse(dbc_struct_i(c)->c, key, c_getf_set_range_reverse_callback, &context);
+        r = toku_ft_cursor_set_range_reverse(dbc_ftcursor(c), key, c_getf_set_range_reverse_callback, &context);
         if (r == DB_LOCK_NOTGRANTED) {
             r = toku_db_wait_range_lock(context.base.db, context.base.txn, &context.base.request);
         } else {
@@ -710,11 +710,10 @@ c_getf_set_range_reverse_callback(ITEMLEN keylen, bytevec key, ITEMLEN vallen, b
 }
 
 // Close a cursor.
-int 
-toku_c_close(DBC * c) {
+int toku_c_close(DBC *c) {
     HANDLE_PANICKED_DB(c->dbp);
     HANDLE_CURSOR_ILLEGAL_WORKING_PARENT_TXN(c);
-    toku_ft_cursor_close(dbc_struct_i(c)->c);
+    toku_ft_cursor_destroy(dbc_ftcursor(c));
     toku_sdbt_cleanup(&dbc_struct_i(c)->skey_s);
     toku_sdbt_cleanup(&dbc_struct_i(c)->sval_s);
     toku_free(c);
@@ -740,7 +739,7 @@ c_set_bounds(DBC *dbc, const DBT *left_key, const DBT *right_key, bool pre_acqui
     DB *db = dbc->dbp;
     DB_TXN *txn = dbc_struct_i(dbc)->txn;
     HANDLE_PANICKED_DB(db);
-    toku_ft_cursor_set_range_lock(dbc_struct_i(dbc)->c, left_key, right_key,
+    toku_ft_cursor_set_range_lock(dbc_ftcursor(dbc), left_key, right_key,
                                    (left_key == toku_dbt_negative_infinity()),
                                    (right_key == toku_dbt_positive_infinity()),
                                    out_of_range_error);
@@ -758,12 +757,12 @@ c_set_bounds(DBC *dbc, const DBT *left_key, const DBT *right_key, bool pre_acqui
 
 static void
 c_remove_restriction(DBC *dbc) {
-    toku_ft_cursor_remove_restriction(dbc_struct_i(dbc)->c);
+    toku_ft_cursor_remove_restriction(dbc_ftcursor(dbc));
 }
 
 static void
 c_set_check_interrupt_callback(DBC* dbc, bool (*interrupt_callback)(void*), void *extra) {
-    toku_ft_cursor_set_check_interrupt_cb(dbc_struct_i(dbc)->c, interrupt_callback, extra);
+    toku_ft_cursor_set_check_interrupt_cb(dbc_ftcursor(dbc), interrupt_callback, extra);
 }
 
 int
@@ -842,8 +841,6 @@ toku_db_cursor_internal(DB * db, DB_TXN * txn, DBC ** c, uint32_t flags, int is_
             );
     }
 
-    int r = 0;
-    
     struct __toku_dbc_external *XMALLOC(eresult); // so the internal stuff is stuck on the end
     memset(eresult, 0, sizeof(*eresult));
     DBC *result = &eresult->external_part;
@@ -889,25 +886,25 @@ toku_db_cursor_internal(DB * db, DB_TXN * txn, DBC ** c, uint32_t flags, int is_
         is_snapshot_read = (dbc_struct_i(result)->iso == TOKU_ISO_READ_COMMITTED || 
                             dbc_struct_i(result)->iso == TOKU_ISO_SNAPSHOT);
     }
-    r = toku_ft_cursor(
+    int r = toku_ft_cursor_create(
         db->i->ft_handle, 
-        &dbc_struct_i(result)->c,
+        dbc_ftcursor(result),
         txn ? db_txn_struct_i(txn)->tokutxn : NULL,
         is_snapshot_read,
         ((flags & DBC_DISABLE_PREFETCHING) != 0)
         );
-    assert(r == 0 || r == TOKUDB_MVCC_DICTIONARY_TOO_NEW);
     if (r == 0) {
         // Set the is_temporary_cursor boolean inside the ftnode so
         // that a query only needing one cursor will not perform
         // unecessary malloc calls.
+        //
+        // TODO: Move me to toku_ft_cursor_create constructor
         if (is_temporary_cursor) {
-            toku_ft_cursor_set_temporary(dbc_struct_i(result)->c);
+            toku_ft_cursor_set_temporary(dbc_ftcursor(result));
         }
-
         *c = result;
-    }
-    else {
+    } else {
+        invariant(r == TOKUDB_MVCC_DICTIONARY_TOO_NEW);
         toku_free(result);
     }
     return r;