Arc Forumnew | comments | leaders | submitlogin
'disktem
4 points by evanrmurphy 5077 days ago | 8 comments
"arc.arc" has 'diskvar for variables and 'disktable for tables. This is the same thing for templates:

  (mac disktem (var tem file)
    (w/uniq (gf gv)
      `(fromdisk ,var ,file (inst ',tem) [temload ',tem _] (fn (,gv ,gf)
                                                             (writefile (tablist ,gv) ,gf)))))
It's working on preliminary tests, but I need to do more, so let me flag its reliability "tentative" for the moment.

Questions:

- Is this a case that could have used akkartik's 'defgeneric [1] or aw's 'extend [2] ?

- Is palsecam's 'ptable [3] only for tables or does it work with other types as well?

[1] http://arclanguage.org/item?id=11779

[2] http://awwx.ws/extend0

[3] http://arclanguage.org/item?id=10664



2 points by akkartik 5076 days ago | link

I've been moving toward a serialize/unserialize/persisted triad. serialize and unserialize are generics[1].

  (defgeneric serialize(x)
    x)

  (defmethod serialize(x) table
    (list 'table
      (accum a
        (maptable (fn (k v)
                    (a (list k serialize.v)))
                  x))))
  (pickle table serialize)
You can't define unserialize with defgeneric, but once defined you can extend it with defmethod:

  ; can't use defgeneric; everything is likely a list when serialized
  (or= vtables*!unserialize (table))
  (def unserialize(x)
    (aif (vtables*!unserialize type*.x)
      (it x)
      x))
  (defmethod unserialize(x) cons
    (map unserialize x))

  (defmethod unserialize(x) table
    (w/table h
      (map (fn ((k v)) (= h.k unserialize.v))
           cadr.x)))

  (def type*(x)
    (if (and (pair? x)
             (isa car.x 'sym))
      car.x
      type.x))
This is a better approach than implicitly converting tables to alists and templates to tables, IMO. I kept needing to futz around with corner cases when reading complex data structures. If I see a nil is it a table or a list in that position? etc., etc.

persisted is similar to palsecam's ptable, except it uses serialize/unserialize to mix in persistence for any datatype; I'll publish it at some point.

I imagine you could make diskvar generic in similar vein. Then you won't need disktable and disktem. These variants are a wart on the face of arc :)

[1] I've changed the order of args in defmethod since http://arclanguage.org/item?id=11779

-----

2 points by akkartik 5072 days ago | link

Here's my implementation of persisted.

  (mac fread(filename val)
    (let f (uniq)
      `(w/infile ,f ,filename
          (= ,val (unserialize:read ,f)))))

  (def fwrite(filename val)
    (w/outfile f filename
      (write serialize.val f)))

  (def fwritefile(filename val)
    (let tmpfile (+ filename ".tmp")
      (fwrite tmpfile val)
      (until file-exists.tmpfile)
      (mvfile tmpfile filename)))

  (mac load-snapshot(var initval)
    `(aif (file-exists ,(stringify var))
        (fread it ,var)
        (= ,var ,initval)))

  (mac save-snapshot(var)
    `(fwritefile ,(stringify var) ,var))

  (= persisted-vars* ())
  (mac persisted(var initval)
    `(do
       (load-snapshot ,var ,initval)
       (pushnew ',var persisted-vars*)))
I'm less certain about the part that actually does the autosaving; you'll want to adjust it for your situation. Here's what works for me:

  (def autosave-state()
    (while t
      (each var persisted-vars*
        (eval `(save-snapshot ,var)))
      (sleep 300)))
  (new-thread autosave-state)
This is less efficient that ptable, but ptable doesn't work for nested tables; you can't detect writes to inner tables.

-----

4 points by fallintothis 5070 days ago | link

Sorry for the unsolicited code review (I don't mean to be malicious), but the implementation seems overcomplicated: wheel reinvention that's inconsistent with Arc's built-ins.

Why is fread a macro? You can easily set a variable explicitly when you want it. Moreover, the basic functionality is like Arc's readfile1, not read. It can simplify to a composition of two functions:

  (def freadfile1 (name)
    (unserialize (readfile1 name)))
If you wanted something more like readfile that unserializes each form, you can just amend Arc's definition:

  (def freadfile (name)
    (w/infile s name (drain (unserialize (read s)))))
I suppose there's (map unserialize (readfile name)), but that's an extra pass with more consing.

fwrite and fwritefile appear to duplicate functionality -- one just makes a temp file first. I assume you got the idea from Arc's writefile, except you added (until file-exists.tmpfile). Is there a reason for this? Did you have issues with trying to move the file before it was finished writing?

That point notwithstanding, they can still be rewritten closer to Arc's built-ins:

  (def fwrite (val (o output (stdout)))
    (write (serialize val) output))

  (def fwritefile (val name)
    (writefile (serialize val) name))
The f prefixing all these names confuses me. What is it supposed to stand for? "File"?

load-snapshot uses stringify, which I assume is supposed to be Arc's string (which works just fine for converting symbols to strings). It also has a variable capture issue since it uses aif. E.g., using your original definition with (= stringify string):

  arc> (load "macdebug.arc") ; see http://www.arclanguage.org/item?id=11806
  arc> (macsteps '(load-snapshot it init) 'show 'load-snapshot 'aif)
  Showing only: load-snapshot, aif

  Expression:

    (load-snapshot it init)

  Macro Expansion:

      (load-snapshot it init)

  ==> (aif (file-exists "it")
           (fread it it)
           (= it init))

  Expression:

    (aif (file-exists "it")
         (fread it it)
         (= it init))

  Macro Expansion:

      (aif (file-exists "it")
           (fread it it)
           (= it init))

  ==> (let it (file-exists "it")
        (if it (fread it it) (= it init)))

  Expression:

    (let it (file-exists "it")
      (if it (fread it it) (= it init)))

  nil
Really, what's happening between load-snapshot and save-snapshot is encapsulated by Arc's fromdisk and todisk (added benefit: no variable capture).

  (mac load-snapshot (var init)
    `(fromdisk ,var ,(string var) ,init freadfile1 fwritefile))

  (mac save-snapshot (var)
    `(todisk ,var))
persisted can also use do1 to be more like fromdisk. Instead of returning the list of persisted variables, it will return the actual variable's value. I don't know if your behavior was intentional, but it's an easy change.

  (= persisted-vars* nil)

  (mac persisted (var init)
    `(do1 (load-snapshot ,var ,init)
          (pushnew ',var persisted-vars*)))
Finally, why not make autosave-state thread itself, instead of having to remember to call new-thread?

  (def autosave-state ()
    (thread
      (while t
        (each var persisted-vars*
          (eval `(save-snapshot ,var)))
        (sleep 300))))
Altogether, that's:

  (def freadfile1 (name)
    (unserialize (readfile1 name)))

  (def fwritefile (val name)
    (writefile (serialize val) name))

  (mac load-snapshot (var init)
    `(fromdisk ,var ,(string var) ,init freadfile1 fwritefile))

  (mac save-snapshot (var)
    `(todisk ,var))

  (= persisted-vars* nil)

  (mac persisted (var init)
    `(do (load-snapshot ,var ,init)
         (pushnew ',var persisted-vars*)))

  (def autosave-state ()
    (thread
      (while t
        (each var persisted-vars*
          (eval `(save-snapshot ,var)))
        (sleep 300))))
This is more consistent with Arc's counterparts and makes good use of existing functionality. Indeed, load-snapshot is Arc's diskvar, except we lace it with serialize & unserialize to handle different structures generically, which is useful.

It could be shorter if we accept freadfile1, fwritefile, load-snapshot, and save-snapshot as implementation details of persisted:

  (= persisted-vars* nil)

  (mac persisted (var init)
    (w/uniq (val file)
      `(do1 (fromdisk ,var
                      ,(string var)
                      ,init
                      unserialize:readfile1
                      (fn (,val ,file) (writefile (serialize ,val) ,file)))
            (pushnew ',var persisted-vars*))))

  (def autosave-state ()
    (thread
      (while t
        (each var persisted-vars*
          (eval `(todisk ,var)))
        (sleep 300))))
Anyway, I think the autosaving and un/serialize generic combo is pretty cool. It lets you extend the functionality more than Arc's existing fromdisk stuff, since you just make a new generic function to handle your data; then you get to keep using persisted.

-----

2 points by akkartik 5069 days ago | link

Thanks a lot. My implementation is an accretion since I first started playing with arc. I no longer remember what the 'f' prefix stands for :) Perhaps it should be gread and gwrite for generic or generalized..

I'm going to spend a while going over the methods you pointed out - I still wasn't familiar with readfile1 or fromdisk. Thanks. Code review is always appreciated.

Update: I'm reminded of an earlier bit of code review from you that was super useful as well: http://arclanguage.org/item?id=10700

Also, folks may find it interesting to compare your solution and mine with a version from several months ago: http://arclanguage.org/item?id=10699

Update 2: Ah, fromdisk isn't in the arclanguage.org documentation! That helps me understand where my efforts have been lacking.

-----

1 point by akkartik 5069 days ago | link

"you added (until file-exists.tmpfile). Is there a reason for this? Did you have issues with trying to move the file before it was finished writing?"

Just looked through my changelogs. The answer is yes, I seem to have run into this issue at some point. writing is a disk-level creature where mv is a file-system-level beast. That seems to cause occasional issues.

-----

2 points by evanrmurphy 5077 days ago | link

Hmmm... I was going to say it'd be nicer to put that save function in its own utility for a tidier 'disktem:

  (def temsave (tem file)
    (writefile (tablist tem) file))

  (mac disktem (var tem file)
    `(fromdisk ,var ,file (inst ',tem) [temload ',tem _] temsave))
Which you can do, but that 'temsave is isomorphic to (the already-existing) 'save-table:

  (def save-table (h file)
    (writefile (tablist h) file))
So could 'disktem be rewritten as the following?

  (mac disktem (var tem file)
    `(fromdisk ,var ,file (inst ',tem) [temload ',tem _] save-table))
If so, it has taught me something about templates: they're simply tables on writes/saves; it's on reads/loads that their difference manifests (as far as I/O is concerned, anyway). [1]

[1] And the same can be said for tables and alists, since tables get 'tablist-ed before they're written/saved. Neat!

-----

1 point by evanrmurphy 5075 days ago | link

Minor revision of that last 'disktem definition:

  (mac disktem (var tem file)
    `(fromdisk ,var ,file (inst ,tem) [temload ,tem _] save-table))
The only difference is ,tem instead of ',tem for consistency with the other template utilities, which don't quote it.

Did more testing and the reliability seems good, in case anyone else wants this big ol' wart for their own arc's face. ;)

-----

3 points by evanrmurphy 5077 days ago | link

How does 'temloadall play into this? Would you need a "'disktem-all" using that instead of 'temload to cover all the cases?

-----