From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id CA77430126 for ; Tue, 18 Jun 2019 16:39:41 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id g5EQEKQlpEvc for ; Tue, 18 Jun 2019 16:39:41 -0400 (EDT) Received: from smtp52.i.mail.ru (smtp52.i.mail.ru [94.100.177.112]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 503642D78B for ; Tue, 18 Jun 2019 16:39:41 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v1 26/28] sql: cleanup of legacy memory management system References: <83a31a15dfe41460a572961ae55540ebb41e8c3e.1560174553.git.imeevma@gmail.com> <87ff4cf8-d816-8e97-be34-e4272e9a3218@tarantool.org> <20190615100444.GI32365@tarantool.org> From: Vladislav Shpilevoy Message-ID: Date: Tue, 18 Jun 2019 22:40:09 +0200 MIME-Version: 1.0 In-Reply-To: <20190615100444.GI32365@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: Mergen Imeev Cc: tarantool-patches@freelists.org 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 ``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 - * 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