On 07.02.2011 21:23, Johan Corveleyn wrote:
> On Mon, Feb 7, 2011 at 5:28 PM, Stefan Sperling <stsp_at_elego.de> wrote:
>> On Mon, Feb 07, 2011 at 05:18:57PM +0100, Stefan Sperling wrote:
>>> On Mon, Feb 07, 2011 at 10:55:47AM -0500, Mark Phippard wrote:
>>>> On Mon, Feb 7, 2011 at 10:52 AM, Stefan Sperling <stsp_at_elego.de> wrote:
>>>>
>>>>> Where is the temporary table stored? Is it back by a file or memory?
>>>>> If backed by memory, do we have to worry about memory consumption for
>>>>> large working copies?
>>>> The patch says it is backed by a file.
>>> Yes, it does say that in a comment but I didn't see where this is being
>>> enforced in the code.
>>> Checking the sqlite docs gave the answer:
>>>
>>> "When the name of the database file handed to sqlite3_open() or to ATTACH
>>> is an empty string, then a new temporary file is created to hold the
>>> database."
>>> http://sqlite.org/inmemorydb.html
>>>
>>> This is what the patch does:
>>>
>>> +-- STMT_ATTACH_TEMPORARY_DATABASE
>>> +ATTACH DATABASE '' AS temp_query_cache;
>>>
>>> And note that the sqlite docs also say:
>>>
>>> "Even though a disk file is allocated for each temporary database, in
>>> practice the temporary database usually resides in the in-memory pager
>>> cache and hence is very little difference between a pure in-memory
>>> database created by ":memory:" and a temporary database created by an
>>> empty filename. The sole difference is that a ":memory:" database must
>>> remain in memory at all times whereas parts of a temporary database
>>> might be flushed to disk if database becomes large or if SQLite comes
>>> under memory pressure."
>>>
>>> Neat :)
>> Actually, we are forcing all temporary databases to be pure memory
>> in svn_sqlite__open():
>>
>> /* Store temporary tables in RAM instead of in temporary files, but don't
>> fail on this if this option is disabled in the sqlite compilation by
>> setting SQLITE_TEMP_STORE to 0 (always to disk) */
>> svn_error_clear(exec_sql(*db, "PRAGMA temp_store = MEMORY;"));
>>
>> Is this really a good idea? I think we should set it to FILE by default.
>> http://sqlite.org/pragma.html#pragma_temp_store
> I've been wondering about the question "how about storing/buffering
> the entire query results in memory?" Would this really be a problem,
> even for very large working copies?
Dunno. Note that, knowing about the pragma for temp tables, I
intentionally created a temporar /database/, not a temporary table --
hence the pragma didn't catch, and indeed sqlite did create a backing
file somewhere in /var/tmp for me. :) However, it also made the code
frustratingly more complex than necessary.
On the subject of temp tables in general: I think we should stick to
having them file-backed, not in-memory. The latter helps
performance-wise only because wc-ng is currently about as efficient in
its use of sqlite as using a flamethrower would be to slap a mosquito
... too many transactions, too many queries, potentially too many
instantiations of temp tables.
Instead, the complex operations should be rewritten in the same format
as this patch of mine, then creating and populating file-backed temp
tables would not be a performance issue, plus the actual code would be
much simpler since one wouldn't have to juggle with temporary databases
in order to get the right semantics from the implementtion.
By the way, I've been wondering why actual_nodes is separate from nodes
.. it makes the kind of query you usually need much more complex than
necessary, since you have to merge actual_nodes into the results of a
nodes query every time. Why not merge them in the database, and assign
another op_depth to what used to be actual_nodes? Perhaps in 1.8?
-- Brane
-- Brane
Received on 2011-02-07 21:43:17 CET