Hi Christophe, thanks for mulling over my changesets.

The email saying "Nice compiler speedup" on Sep 17th contains a completely working patch. I have minor touch-ups that are not strictly needed, and I mean really minor - most notably a comment containing a prototypical packed info representation is wrong, because I changed the representation without pasting in a new comment.  I could mail you a complete patch with touch-ups; but to keep things simple, you could review the patch as-is and provide feedback. No need to stir the pot any more at this point.

If it's all the same to you, that patch hangs together as one conceptual thing, however it does have sub-pieces that can be factored out if you like:

* The non-self-rebuild was my fault which of course I needed a working cold-print to track down.  The aforementioned patch can self-rebuild fine. It was a crash on the road to getting StaticSymbolFunction() not to be special-cased. As you know, that C code currently only works for symbols whose fdefinition is in their value cell (as well as globaldb). My representation facilitated removing that limitation, so I did. Technically I needn't have touched C code at all though, if only I had preserved the behavior that WHN deemed "weird".

* WHN's comment that it is ugly that DEFINE-INFO-TYPE takes a :DEFAULT form which assumes the existence of a lexical var named NAME, which may be used to decide what an info's default value should be.  This was somewhat orthogonal to anything else, but I felt like correcting the lack of hygiene.

* remark, sans fix, that SETF-ASSUMED-FBOUNDP is a bit silly - just a few lines to add a comment, so why not? ("Monsieur, it's wafer thin ...")
 
You can stop reading now and just go review that (thanks!), but here are remaining issues I came across though, none of which are incorporated into the large patch, and some of which I sent subsequent emails about:

- The fact that clear-info-value doesn't work means that the second return value of GET-INFO-VALUE (aka the WINP/FOUNDP value) is genuinely wrong sometimes, because at best you can set a value back to NIL, but not truly clear it from a compact env. If the big patch is done along with my subsequent proposal to use a quasi-lock-free unified hashtable for "complicated" names, we can then go through 'info-functions' and see what else should have been cleared instead of set to NIL. e.g. the logic in UNDEFINE-FUN-NAME in src/compiler/info-functions *knows* that clearing doesn't actually work as advertised, and it's secretly using the workaround without saying so.

- The general issue (which I never emailed to sbcl-devel but only in conversation with Nikodemus) of method names that have an EQL in them generate several different "names" in globaldb - potentially the cross-product of each of {'fast-method,'slow-method} with the value of the EQL form {before, after} it has been evaled.  We should eradicate all infos from bogus names. One way to do that is that method functions should be anonymous lambdas until the putative name of such lambda has had all (if any) EQL specializers interned, because otherwise every source-form used to define every EQL specializer is needlessly part of a (useless) name in globaldb.

- The poor hashing of list-shaped PCL method names due to sxhashoid's depthoid cutting off in both the CAR and CDR axis (with or without a fix to the preceding issue, this is still an issue)

- Leftover detritus from cross-compilation remains in target Lisp as the :xc-constant-value info-type.  I have made that not exist in the target.

- A comment "FIXME: is it too expensive to go through a runtime call to FDEFINITION each time we want to check a name's syntax?" can be dealt with by storing the function that computes a name's validity, not the symbol naming the function that computes the name's validity. There's a bootstrapping issue afaict. Note that my implementation of fdefinition-object avoids calling legal-fun-name-or-type-error until it absolutely has to due to a "miss" in the globaldb for the fdefinition of the name being sought.

 [Also before anyone mentions that the hashtable algorithm (in the email of Oct 1) doesn't support key removal: indeed, but you don't need key removal, you only need removal of an info-type-number which would be in the packed-info-vector stored under a key's value, and that is supported by, in the limiting case, having a vector of no info.]

I hope all is clear now.
Regards. -d



On Mon, Oct 7, 2013 at 8:36 AM, Christophe Rhodes <csr21@cantab.net> wrote:
Douglas Katzman <dougk@google.com> writes:

> Cold-print accidentally didn't print the recursed-into object in its
> fallback case, but instead the outer argument. Hexstr didn't work on
> 64-bit.  Both for "hapless wretches" [sic].

Thanks.  I've incorporated your fixes (by hand, sorry, so there may be
conflicts).

I'm not ignoring the big globaldb issue, but I have slightly lost track,
partly because as a hapless wretch you have managed to trigger issue
after issue :-) so maybe we could recap -- is the current status clearer
now?  The message I'm replying to refers to an inability to
self-rebuild; has that been fixed?  What's the latest thing that I
should be reviewing?

Thanks,

Christophe