[tarantool-patches] Re: [PATCH v1 26/28] sql: cleanup of legacy memory management system
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Tue Jun 18 23:40:09 MSK 2019
Thanks for the squash. I've found one another part of
SQL memory subsystem that can be removed. Please, consider
my review fixes here and on the branch in a separate commit.
======================================================
diff --git a/src/box/sql/CMakeLists.txt b/src/box/sql/CMakeLists.txt
index f0b2e78de..1f2a6640f 100644
--- a/src/box/sql/CMakeLists.txt
+++ b/src/box/sql/CMakeLists.txt
@@ -31,7 +31,6 @@ add_library(sql STATIC
date.c
delete.c
expr.c
- fault.c
fk_constraint.c
func.c
global.c
diff --git a/src/box/sql/fault.c b/src/box/sql/fault.c
deleted file mode 100644
index ee3a4ea6d..000000000
--- a/src/box/sql/fault.c
+++ /dev/null
@@ -1,91 +0,0 @@
-/*
- * Copyright 2010-2017, Tarantool AUTHORS, please see AUTHORS file.
- *
- * Redistribution and use in source and binary forms, with or
- * without modification, are permitted provided that the following
- * conditions are met:
- *
- * 1. Redistributions of source code must retain the above
- * copyright notice, this list of conditions and the
- * following disclaimer.
- *
- * 2. Redistributions in binary form must reproduce the above
- * copyright notice, this list of conditions and the following
- * disclaimer in the documentation and/or other materials
- * provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
- * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
- * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
- * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
- * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
- * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
- * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- */
-
-/*
- *
- * This file contains code to support the concept of "benign"
- * malloc failures (when the xMalloc() or xRealloc() method of the
- * sql_mem_methods structure fails to allocate a block of memory
- * and returns 0).
- *
- * Most malloc failures are non-benign. After they occur, sql
- * abandons the current operation and returns an error
- * to the user. However, sometimes a fault is not necessarily
- * fatal. For example, if a malloc fails while resizing a hash table, this
- * is completely recoverable simply by not carrying out the resize. The
- * hash table will continue to function normally. So a malloc failure
- * during a hash table resize is a benign fault.
- */
-
-#include "sqlInt.h"
-
-/*
- * Global variables.
- */
-typedef struct BenignMallocHooks BenignMallocHooks;
-static SQL_WSD struct BenignMallocHooks {
- void (*xBenignBegin) (void);
- void (*xBenignEnd) (void);
-} sqlHooks = {
-0, 0};
-
-/* The "wsdHooks" macro will resolve to the appropriate BenignMallocHooks
- * structure. If writable static data is unsupported on the target,
- * we have to locate the state vector at run-time. In the more common
- * case where writable static data is supported, wsdHooks can refer directly
- * to the "sqlHooks" state vector declared above.
- */
-#define wsdHooksInit
-#define wsdHooks sqlHooks
-
-/*
- * This (sqlEndBenignMalloc()) is called by sql code to indicate that
- * subsequent malloc failures are benign. A call to sqlEndBenignMalloc()
- * indicates that subsequent malloc failures are non-benign.
- */
-void
-sqlBeginBenignMalloc(void)
-{
- wsdHooksInit;
- if (wsdHooks.xBenignBegin) {
- wsdHooks.xBenignBegin();
- }
-}
-
-void
-sqlEndBenignMalloc(void)
-{
- wsdHooksInit;
- if (wsdHooks.xBenignEnd) {
- wsdHooks.xBenignEnd();
- }
-}
-
diff --git a/src/box/sql/hash.c b/src/box/sql/hash.c
index 5bc8109dc..36f965b81 100644
--- a/src/box/sql/hash.c
+++ b/src/box/sql/hash.c
@@ -149,18 +149,7 @@ rehash(Hash * pH, unsigned int new_size)
if (new_size == pH->htsize)
return 0;
#endif
-
- /* The inability to allocates space for a larger hash table is
- * a performance hit but it is not a fatal error. So mark the
- * allocation as a benign. Use sqlMalloc()/memset(0) instead of
- * sqlMallocZero() to make the allocation, as sqlMallocZero()
- * only zeroes the requested number of bytes whereas this module will
- * use the actual amount of space allocated for the hash table (which
- * may be larger than the requested amount).
- */
- sqlBeginBenignMalloc();
new_ht = (struct _ht *)sqlMalloc(new_size * sizeof(struct _ht));
- sqlEndBenignMalloc();
if (new_ht == 0)
return 0;
diff --git a/src/box/sql/malloc.c b/src/box/sql/malloc.c
index f19b7d027..d92709b8e 100644
--- a/src/box/sql/malloc.c
+++ b/src/box/sql/malloc.c
@@ -368,17 +368,6 @@ sqlDbStrNDup(sql * db, const char *z, u64 n)
return zNew;
}
-/*
- * Call this routine to record the fact that an OOM (out-of-memory) error
- * has happened. This routine will set db->mallocFailed.
- */
-void
-sqlOomFault(sql * db)
-{
- if (db->mallocFailed == 0 && db->bBenignMalloc == 0)
- db->mallocFailed = 1;
-}
-
/*
* This routine reactivates the memory allocator and clears the
* db->mallocFailed flag as necessary.
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 2e7bf3363..e4c584333 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -1107,7 +1107,6 @@ struct sql {
u16 dbOptFlags; /* Flags to enable/disable optimizations */
u8 enc; /* Text encoding */
u8 mallocFailed; /* True if we have seen a malloc failure */
- u8 bBenignMalloc; /* Do not require OOMs if true */
u8 dfltLockMode; /* Default locking-mode for attached dbs */
u8 mTrace; /* zero or more sql_TRACE flags */
u32 magic; /* Magic number for detect library misuse */
@@ -4290,7 +4289,14 @@ int sqlCreateFunc(sql *, const char *, enum field_type,
void (*)(sql_context *, int, sql_value **),
void (*)(sql_context *),
FuncDestructor * pDestructor);
-void sqlOomFault(sql *);
+
+/** Set OOM error flag. */
+static inline void
+sqlOomFault(struct sql *db)
+{
+ db->mallocFailed = 1;
+}
+
void sqlOomClear(sql *);
int sqlApiExit(sql * db, int);
@@ -4472,13 +4478,6 @@ fk_constraint_emit_actions(struct Parse *parser, struct space *space, int reg_ol
bool
fk_constraint_is_required(struct space *space, const int *changes);
-/*
- * The interface to the code in fault.c used for identifying "benign"
- * malloc failures.
- */
-void sqlBeginBenignMalloc(void);
-void sqlEndBenignMalloc(void);
-
/*
* Allowed return values from sqlFindInIndex()
*/
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 579b2006c..c8887f9b7 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -747,7 +747,6 @@ int sqlVdbeExec(Vdbe *p)
assert(p->explain==0);
p->pResultSet = 0;
#ifdef SQL_DEBUG
- sqlBeginBenignMalloc();
if (p->pc == 0 &&
(p->sql_flags & (SQL_VdbeListing|SQL_VdbeEQP|SQL_VdbeTrace)) != 0) {
int i;
@@ -771,7 +770,6 @@ int sqlVdbeExec(Vdbe *p)
if ((p->sql_flags & SQL_VdbeTrace) != 0)
printf("VDBE Trace:\n");
}
- sqlEndBenignMalloc();
#endif
for(pOp=&aOp[p->pc]; 1; pOp++) {
/* Errors are detected by individual opcodes, with an immediate
More information about the Tarantool-patches
mailing list