From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Subject: Re: Cuirass news Date: Fri, 09 Feb 2018 17:53:05 +0100 Message-ID: <877ermyuj2.fsf@gnu.org> References: <877es6x5xj.fsf@gnu.org> <87lggmjjgo.fsf@gmail.com> <87k1w6jjak.fsf@gmail.com> <87h8raxeym.fsf@gnu.org> <20180126153005.259a75e8@scratchpost.org> <87zi4z1eb0.fsf@gnu.org> <20180127181852.42f0bcbf@scratchpost.org> <87fu6bwqix.fsf@gnu.org> <20180208172905.19e9e789@scratchpost.org> <871shvt94p.fsf@gnu.org> <20180209000526.5b9ea8e7@scratchpost.org> <87wozm4i12.fsf@gnu.org> <20180209122931.5a47e63e@scratchpost.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:39794) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ekBvF-0001Ff-3f for guix-devel@gnu.org; Fri, 09 Feb 2018 11:53:14 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ekBvB-0005KR-UJ for guix-devel@gnu.org; Fri, 09 Feb 2018 11:53:13 -0500 Received: from hera.aquilenet.fr ([2a0c:e300::1]:57246) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1ekBvB-0005GW-HI for guix-devel@gnu.org; Fri, 09 Feb 2018 11:53:09 -0500 In-Reply-To: <20180209122931.5a47e63e@scratchpost.org> (Danny Milosavljevic's message of "Fri, 9 Feb 2018 12:29:31 +0100") List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Danny Milosavljevic Cc: guix-devel --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Heya, Danny Milosavljevic skribis: > On Fri, 09 Feb 2018 10:41:13 +0100 > ludo@gnu.org (Ludovic Court=C3=A8s) wrote: [...] >> >> Indeed! Should we change =E2=80=98sqlite-finalize=E2=80=99 to a noop= when called on a >> >> cached statement? (Otherwise users would have to keep track of wheth= er >> >> or not a statement is cached.)=20=20 >> > >> > Hmm maybe that's a good way. But its a little magic.=20=20 >>=20 >> Yes, but I think we have no other option: now that caching is built into >> sqlite3.scm, it has to be properly handled by all of that module. For >> the user, it should be a simple matter of choosing #:cache? #t >> or #:cache? #f, and then (sqlite3) should just DTRT. > > Yeah, but then let's add sqlite-uncache or something that can be used to > remove it from the cache after all. And make sqlite-finalize a noop if > it's cached. Sounds good. What about this patch: --=-=-= Content-Type: text/x-patch; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable diff --git a/sqlite3.scm b/sqlite3.scm index fa96bdb..e8d2bf8 100644 --- a/sqlite3.scm +++ b/sqlite3.scm @@ -1,5 +1,6 @@ ;; Guile-SQLite3 ;; Copyright (C) 2010, 2014 Andy Wingo +;; Copyright (C) 2018 Ludovic Court=C3=A8s =20 ;; This library is free software; you can redistribute it and/or modify ;; it under the terms of the GNU Lesser General Public License as @@ -114,6 +115,14 @@ (open? db-open? set-db-open?!) (stmts db-stmts)) =20 +(define-record-type + (make-stmt pointer live? reset? cached?) + stmt? + (pointer stmt-pointer) + (live? stmt-live? set-stmt-live?!) + (reset? stmt-reset? set-stmt-reset?!) + (cached? stmt-cached? set-stmt-cached?!)) + (define sqlite-errmsg (let ((f (pointer->procedure '* @@ -145,11 +154,17 @@ (dynamic-func "sqlite3_close" libsqlite3) (list '*)))) (lambda (db) - (if (db-open? db) - (begin - (let ((p (db-pointer db))) - (set-db-open?! db #f) - (f p))))))) + (when (db-open? db) + ;; Finalize cached statements. + (hash-for-each (lambda (sql stmt) + (set-stmt-cached?! stmt #f) + (sqlite-finalize stmt)) + (db-stmts db)) + (hash-clear! (db-stmts db)) + + (let ((p (db-pointer db))) + (set-db-open?! db #f) + (f p)))))) =20 (define db-guardian (make-guardian)) (define (pump-db-guardian) @@ -208,18 +223,11 @@ (ele (db-pointer db) onoff)))) =20 =20 + ;;; ;;; SQL statements ;;; =20 -(define-record-type - (make-stmt pointer live? reset? cached?) - stmt? - (pointer stmt-pointer) - (live? stmt-live? set-stmt-live?!) - (reset? stmt-reset? set-stmt-reset?!) - (cached? stmt-cached?)) - (define sqlite-remove-statement! (lambda (db stmt) (when (stmt-cached? stmt) @@ -240,11 +248,15 @@ (dynamic-func "sqlite3_finalize" libsqlite3) (list '*)))) (lambda (stmt) - (if (stmt-live? stmt) - (let ((p (stmt-pointer stmt))) - (sqlite-remove-statement! (stmt->db stmt) stmt) - (set-stmt-live?! stmt #f) - (f p)))))) + ;; Note: When STMT is cached, this is a no-op. This ensures caching + ;; actually works while still separating concerns: users can turn + ;; caching on and off without having to change the rest of their cod= e. + (when (and (stmt-live? stmt) + (not (stmt-cached? stmt))) + (let ((p (stmt-pointer stmt))) + (sqlite-remove-statement! (stmt->db stmt) stmt) + (set-stmt-live?! stmt #f) + (f p)))))) =20 (define *stmt-map* (make-weak-key-hash-table)) (define (stmt->db stmt) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable ? I=E2=80=99d reluctant to =E2=80=98sqlite-uncache!=E2=80=99 though. I would= think that if users need more sophisticated caching, they can always implement it in their application. I wouldn=E2=80=99t want us to try to be too smart here. WDYT? Thanks, Ludo=E2=80=99. --=-=-=--