From: Leslie P. P. <sk...@vi...> - 2009-04-29 07:58:17
Attachments:
function-documentation.2009-04-29.diff
|
Hello everyone, the attached patch adds a slot to the SIMPLE-FUN primitive object and uses it to record a function's documentation. This relieves the global info db mudheap from at least one of its responsibilities and enables us to store documentation for anonymous functions. Notes: * We can get rid of %FUN-DOC now too, but for this FDOCUMENTATION needs to be rewritten first. I will do this next but it will be a separate patch. * The rename of %define-primitive-object to the more apt %register-primitive-object was made en passant, is self-contained and has no direct relation to this patch (let me know if you would like to have this separately). This patch is just for review right now. Once it passes muster I will send a complete patch for convenient inclusion. Leslie |
From: Christophe R. <cs...@ca...> - 2009-04-29 08:55:05
|
"Leslie P. Polzer" <sk...@vi...> writes: > This patch is just for review right now. Once it passes > muster I will send a complete patch for convenient inclusion. Questions that I raise immediately (without reviewing the patch itself): * By how much does this change the size of the sbcl.core file dumped at the end of compilation? How would this compare with a weak hash table keyed on function objects? * Does this patch allow setting of function documentation for closures or funcallable instances? Best, Christophe |
From: Leslie P. P. <sk...@vi...> - 2009-04-29 09:14:31
|
> * By how much does this change the size of the sbcl.core file dumped > at the end of compilation? old: 25198620 new: 25247772 difference: ~50k > How would this compare with a weak hash table keyed on > function objects? I don't know. Intuitively I'd guess that a weak hash table in the infodb might yield better results in terms of memory. That would have to be verified, but I wouldn't want to do this right now as my resources are limited. > * Does this patch allow setting of function documentation > for closures Yes. > or funcallable instances? Likely. I need to check this. Thanks! Leslie |
From: Christophe R. <cs...@ca...> - 2009-04-29 10:39:03
|
"Leslie P. Polzer" <sk...@vi...> writes: >> * Does this patch allow setting of function documentation >> for closures > > Yes. OK, I don't see how this is, and I don't see a test case for it, either. As far as I can tell, this needs more work. Here are some examples of test cases that probe a little bit more deeply: (let ((x 1)) (defun foo (y) "bar" (incf x y))) (assert (string= (documentation 'foo 'function) "bar")) (assert (string= (documentation #'foo t)) "bar") (assert (string= (setf (documentation 'foo 'function) "baz") "baz")) (assert (string= (documentation 'foo 'function) "baz")) (assert (string= (documentation #'foo t)) "baz") (defun bar (x) (lambda (y) "doc" (incf x y))) (let ((fun (bar 1))) (assert (string= (documentation fun t) "doc")) (assert (string= (setf (documentation fun t) "whatsup") "whatsup")) (assert (string= (documentation fun t) "whatsup")) (assert (string= (documentation (bar 2) t) "doc"))) Best, Christophe |
From: Leslie P. P. <sk...@vi...> - 2009-04-29 10:49:42
|
> OK, I don't see how this is, and I don't see a test case for it, > either. As far as I can tell, this needs more work. > > Here are some examples of test cases that probe a little bit more > deeply: Thanks. As Nikodemus suspected I misunderstood the question (or rather did not take its full implications into account). This would have to be addressed eventually, but considering that the current code does not account for it either I'd rather leave it for later as an additional feature to be implemented. As for the core size, I'm not sure whether the numbers are a problem. Since you asked the question I guess you can judge this better than me. Leslie -- LinkedIn Profile: http://www.linkedin.com/in/polzer Xing Profile: https://www.xing.com/profile/LeslieP_Polzer Blog: http://blog.viridian-project.de/ |
From: Nikodemus S. <nik...@ra...> - 2009-04-29 10:48:48
|
2009/4/29 Leslie P. Polzer <sk...@vi...>: >> How would this compare with a weak hash table keyed on >> function objects? > > I don't know. Intuitively I'd guess that a weak hash table in > the infodb might yield better results in terms of memory. At any rate, as long as weak hash tables have non-linear GC characteristics I don't think we should be adding any more of them to the build -- especially not ones that grow linearly with the size of the application. >> * Does this patch allow setting of function documentation >> for closures > > Yes. I don't think it does -- not in the sense I believe Christophe means: (defun foo (x) (lambda () x)) (let ((f1 (foo 1)) (f2 (foo 2))) (setf (documentation f1 t) "FOO 1" (documentation f2 t) "FOO 2") (equal (documentation f1 t) (documentation f2 t))) will return T, as the closures share the same simple-fun. Similarly for funcallable instances: the only available slot is in the final underlying simple-fun. I don't consider this a problem. Storing function names shares this issue, and I believe both are best fixed at the same time -- which does not have to be now. It could be, but it doesn't have to. All in all, I like this patch, except for the %DEFINE-PRIMITIVE-OBJECT change, which would be a departure from the convention that we implement load-time bits of DEFFOO with %DEFFOO, and compile-time bits with %COMPILER-DEFFOO. If function size increase is a concern, the slot could be merged with XREFS into a single SIMPLE-FUN-INFO slot, which would hold one of (1) NIL when neither documentation nor xref data is present (2) a string when only documentation is present (3) a simple vector when only xrefs are present(3) a CONS holding both when both are present. Cheers, -- Nikodemus |
From: Leslie P. P. <sk...@vi...> - 2009-04-29 10:56:03
|
> All in all, I like this patch, except for the %DEFINE-PRIMITIVE-OBJECT > change, which would be a departure from the convention that we > implement load-time bits of DEFFOO with %DEFFOO, and compile-time bits > with %COMPILER-DEFFOO. Oh, I didn't consider that. It just seemed weird to me in isolation. Let's revert that change then. > If function size increase is a concern, the slot could be merged with > XREFS into a single SIMPLE-FUN-INFO slot, which would hold one of (1) > NIL when neither documentation nor xref data is present (2) a string > when only documentation is present (3) a simple vector when only xrefs > are present(3) a CONS holding both when both are present. I'd rather have this separated for clarity reasons but I see the point of course. I will leave it up to you guys to decide this and then change the patch accordingly. By the way, are all strings in SBCL of type SIMPLE-STRING? Leslie -- LinkedIn Profile: http://www.linkedin.com/in/polzer Xing Profile: https://www.xing.com/profile/LeslieP_Polzer Blog: http://blog.viridian-project.de/ |
From: Christophe R. <cs...@ca...> - 2009-04-29 11:04:10
|
"Leslie P. Polzer" <sk...@vi...> writes: > By the way, are all strings in SBCL of type SIMPLE-STRING? No. Non-simple ones aren't. All strings are of type STRING, though. Best, Christophe |
From: Nikodemus S. <nik...@ra...> - 2009-06-21 10:52:01
|
I've merged something based on Leslie's patch as 1.0.29.24, main differences being: * Use the same slot in SIMPLE-FUN as as XREFs: the common case for anon functions is no docstring, and doing it this way wastes no space in that case. Functions with both XREFs and a docstring are 1 word larger than when using a separate documentation slot, but this seems like a winning tradeoff. * Also grab the docstrings for FLET and LABELS. * Get rid of INFO :FUNCTION :DOCUMENTATION totally. Initially I was going to add separate documentation to closures as well, but since the primary use-case for docstrings are those inline in source code, I decided that it's a waste of space with only marginal benefits -- if someone actually needs it, I think a weak hash table would be appropriate. Not directly related, but I also experimented adding a name slot to closures, but also deemed it not worth it, since it would only help the printer and never the backtraces -- or at least I cannot see how the debugger could sanely get hold of the closure object for a stack frame as opposed to the simple-fun. Cheers, -- Nikodemus |