Some code review needed
in [Lisp]
|
Prev: parenscript this.bgcolor
Next: escape from parenscript
From: Krzysztof Drewniak on 20 Jun 2010 17:54 I am working on a roguelike called Menace of the Mines (http://motm.sourceforge.net/) I was forced to roll my own savefile system as nothing existing was adequate, and I need code review on that system. The saving is taking *way* too long and I need to know if it's the save code or something else. Also, the system is in src/saves.lisp of the trunk ONLY. Krzysztof -- X-Real-Email-With-Antispam: krzysdrewniak at gmail dot com pgp key on keyserver.ubuntu.com and maybe some other place too
From: RG on 20 Jun 2010 21:30 In article <87mxupbcas.fsf(a)krzys-desktop.home>, Krzysztof Drewniak <krzysdrewniakNOSPAM(a)gmai.com> wrote: > I am working on a roguelike called Menace of the Mines > (http://motm.sourceforge.net/) I was forced to roll my own savefile > system as nothing existing was adequate, and I need code review on that > system. The saving is taking *way* too long and I need to know if it's > the save code or something else. Also, the system is in src/saves.lisp > of the trunk ONLY. > > Krzysztof All those EQL methods are probably not helping your performance. Try this: (defun ds (object) (case (caar object) (:list ...) (:vector ...) (:hash ...)) If that doesn't help, or doesn't help enough, try putting the deserialization functions in the plists of the symbols that you use to specify your types (you should not use keywords in this case) and then do: (defun ds (object) (funcall (get (caar object) 'deserializer) (cdar object) (cdr object))) or something like that. rg
From: Rainer Joswig on 21 Jun 2010 03:39 Krzysztof Drewniak <krzysdrewniakNOSPAM(a)gmai.com> wrote: > I am working on a roguelike called Menace of the Mines > (http://motm.sourceforge.net/) I was forced to roll my own savefile > system as nothing existing was adequate, and I need code review on > that > system. The saving is taking *way* too long and I need to know if it's > the save code or something else. Also, the system is in src/saves.lisp > of the trunk ONLY. i would do that slightly different. S-Expressions are already an external data format. There is no need to have (:string "123"), since "123" is already defined to be a string. The s-expression format can be extended by adding a printer for the data and a reader for the data. The reader can be added to a read table. For example a hashtable could be printed as {key1 value1 key2 value2 ...}. Or using #{ ... }. Similar for other things like objects and classes. A reader function set for the character { can then read the table. Common Lisp provides some built-in data syntax. An advanced example is the reader and writer for structures. Writing data can be customized by writing PRINT-OBJECT methods. So insteade inventing your own methods to read/write data and layer that on top of s-expressions, you could extend s-expressions directly. That way you would use the documented Common Lisp functions.
From: Krzysztof Drewniak on 21 Jun 2010 09:46 Rainer Joswig <joswig(a)lisp.de> writes: > Krzysztof Drewniak <krzysdrewniakNOSPAM(a)gmai.com> wrote: >> I am working on a roguelike called Menace of the Mines >> (http://motm.sourceforge.net/) I was forced to roll my own savefile >> system as nothing existing was adequate, and I need code review on >> that >> system. The saving is taking *way* too long and I need to know if it's >> the save code or something else. Also, the system is in src/saves.lisp >> of the trunk ONLY. > > i would do that slightly different. > > S-Expressions are already an external data format. > There is no need to have (:string "123"), since "123" > is already defined to be a string. > > The s-expression format can be extended by adding a printer > for the data and a reader for the data. The reader can be added > to a read table. For example a hashtable could be > printed as {key1 value1 key2 value2 ...}. Or using #{ ... }. > Similar for other things like objects and classes. > A reader function set for the character { can then read the table. > Common Lisp provides some built-in data syntax. > An advanced example is the reader and writer for structures. > Writing data can be customized by writing PRINT-OBJECT > methods. > > So insteade inventing your own methods to read/write data > and layer that on top of s-expressions, you could > extend s-expressions directly. That way you would > use the documented Common Lisp functions. I have now dine as suggested. As the only thing that *really* needed implementation was object, that was done with { type ((SLOT! . VALUE1)) syntax. People with trunk should svn up and notice the immense speed difference. Thanks guys! Krzysztof Drewniak -- X-Real-Email-With-Antispam: krzysdrewniak at gmail dot com pgp key on keyserver.ubuntu.com and maybe some other place too
From: Rainer Joswig on 21 Jun 2010 10:19
Krzysztof Drewniak <krzysdrewniakNOSPAM(a)gmai.com> wrote: > Rainer Joswig <joswig(a)lisp.de> writes: .... >> >> So insteade inventing your own methods to read/write data >> and layer that on top of s-expressions, you could >> extend s-expressions directly. That way you would >> use the documented Common Lisp functions. > I have now dine as suggested. As the only thing that *really* needed > implementation was object, that was done with { type ((SLOT! . > VALUE1)) > syntax. People with trunk should svn up and notice the immense speed > difference. Thanks guys! DEFMETHOD is a top-level macro. It is not good style to use it inside other functions. In your case it is called always when you want to serialize something. But that is not needed. DEFMETHOD only needs to run once to set the definition. The LET also has no useful effect on the DEFMETHOD, since it binds a special variable. If you have the let around the call to the printer, later then running method will see the dynamic value anyway. Besides that it is not necessary to run DEFMETHOD all the time, DEFMETHOD has top-level side effects, like, possibly informing the running Lisp about where its sources are. Anyway, your code looks much shorter now - which is great. |