freenode/#clasp - IRC Chatlog
Search
15:32:53
Bike
i guess if you allocate a funcallable instance and then immediately call it you get undefined effects. but setting it to some basic closure would be nicer
15:38:45
beach
Speaking of which, when I studied the MOP, I came to the conclusion that this aspect of it was designed so that you can avoid an extra indirection.
15:39:27
beach
Like if your functions have some "code" and "environment" slots, then you can copy those to the funcallable-standard-object.
15:42:12
Bike
https://github.com/clasp-developers/clasp/issues/739 we only copied the "code" slot, so if you gave it a closure there was a problem
15:42:29
Bike
drmeister switched it to use the indirection, which is more correct but has a bit of overhead
15:42:40
Bike
the problem with copying both the code and the environment is how to do it thread safety
15:43:28
Bike
and i just had the idea that if the funcallable instance function happens to not use its environment, i.e. is not a closure, we can get away with just copying the "code"
15:43:55
beach
I can do it in SICL, because both the environment and the code are in the rack, so I just allocate a new rack.
15:45:07
Bike
i think we would have trouble with that since other kinds of functions don't have that indirection.
15:47:08
Bike
but yeah, you're definitely right that the mop was written with this in mind... as i recall, in the original actually-portable PCL code one of the implementation hooks was a low level jump, so as to avoid an actual function call, or something like that
15:48:10
beach
I see. I haven't studied PCL very much. As usual, I try to think it through myself first, and it often results in better solutions.
15:50:51
Bike
might be nice if they'd said so more explicitly, though... as-is, it's not immediately obvious why the funcallable instance function isn't just a slot
15:54:54
beach
It is not the only such situation. I guess it must be a tradition with people writing language specifications.
16:02:28
jackdaniel
on the other hand trying to record the intention makes a short description much longer (and not quite understandable). I've tried to write a serie of blog posts about adding new numbers in ECL and things I've learned (and which had to be put in the document) grew a lot and it was too detailed for a tutorial
16:02:48
jackdaniel
putting only code with short explanation wouldn't cut it either (because it wouldn't be worth much)
16:03:31
beach
The Common Lisp HyperSpec is a lot like that as well. I mean, take the entire distinction between the startup environment, the compilation environment, and the evaluation environment. They could have written that it was meant to allow for first-class global environments.
16:23:13
drmeister
Bike: (bclasp-compile nil (generate-dispatcher-from-dtree ... )) returns a #<The BUILT-IN-CLASS CORE:CLOSURE-WITH-SLOTS>
16:26:20
drmeister
(core:closure-length (third (multiple-value-list (clos:get-funcallable-instance-function #'foo)))) -> 0
16:27:42
drmeister
These are just for debugging - I shouldn't use get-funcallable-instance-function anywere that matters.
16:29:56
Bike
oh, but if i remove a slot from funcallable instance, do i have to do anything weird? it's one after the rack
16:30:17
drmeister
If we check if it's a closure with no slots then in that case we can just copy the code.
16:43:08
Bike
removing not_funcallable_entry_point might be weird because of how initialization works in C++. i need food for this
16:43:20
drmeister
The thread safety issues come in if another thread is executing and there is an inconsistency between the entry and the GFUN_DISPATCHER.
16:43:56
Bike
why would there be a thread safety issue? the call goes to the entry_point, which works regardless of the GFUN_DISPATCHER
16:45:56
Bike
ah, wait. if a thread changes it from a non-closure to a closure it has to change both the entry point and the gfun_dispatcher
16:45:56
drmeister
I was thinking if a compiled dispatcher is over written with an interpreted one - then there could be a problem if GFUN_DISPATCHER was out of sync with entry - but DtreeInterpreter_O keeps track of its generic function as does every compiled dispatcher.
16:47:04
drmeister
DtreeInterpreter and generic function dispatchers are ok - it's closures that are the problem.
16:50:03
drmeister
What if I put a spin lock around writing and reading entry/GF_DISCRIMINATOR? That will slow down closures but not generic function calls.
16:50:25
drmeister
That will slow down the case of funcallable-instance calls that contain closures.
16:50:31
karlosz
okay, also managed to get rid of set predecessors now. will get a build timing sometime later
16:54:08
drmeister
How often do people (set-funcallable-instance-function funcinst (let ((x ...)) (lambda ...)))
17:02:00
scymtym
i would expect the instance function to access the instance in many non-generic function cases
17:16:48
Bike
kpoeck_: i haven't updated our use of sicl yet. if you want to force it, just check out sicl master in the contrib/sicl
17:18:39
Bike
if you do check out sicl master, use the cst build - it won't work with the generate-ast build
17:20:34
scymtym
Bike: apart from generic functions, i would expect uses of funcallable instances to revolve around accessing the instance slots from the instance function. otherwise, what's the point compared to an ordinary function?
17:21:13
Bike
when drmeister said "how often" he actually meant like, is it going to be done in a loop, that kind of thing
17:21:30
kpoeck_
So perhaps i just change wscript so point to sicl master and start with distclean configure
17:21:39
scymtym
oh, dispatching on the class could also be a reason, but i can't think of a use-case
17:21:55
karlosz
kpoeck_: what i do is just set a remote and cherry pick my commits onto the existing git repo in clasp
17:23:03
scymtym
but yeah, updating the instance function is probably a rare operation in most scenarios
17:23:34
Bike
yeah. with fastgf it's actually a little bit not, since it can happen from any call to the function, but i think that's our worst case
17:24:35
scymtym
in that case the cost of compiling the new instance function should dwarf the cost of installing it
17:27:14
karlosz
kpoeck_: if you end up running it, i would appreciate any flame graphs you can get out of it. i have two more fixes waiting as well which should drop the build time even more
17:27:24
scymtym
https://www.felixcloutier.com/x86/cmpxchg8b:cmpxchg16b could be used to update two slots atomically
17:28:08
Bike
yeah, i was wondering about double cas, but i don't think we can do that in llvm easily
17:30:08
scymtym
Bike: yeah, it seems so useful at first sight, but fundamentally relying on such an operation might lock you out of certain platforms later
17:31:17
drmeister
I can generate flame graphs once I have your changes karlosz - Bike is integrating them.
17:31:45
kpoeck
Rebuilding non cst I get: ./generated/initClassesAndMethods_inc.h:10888:71: error: no member named 'call_count' in 'core::DtreeInterpreter_O'
17:31:56
kpoeck
.def(core::magic_name("CORE:call_count"),&DtreeInterpreter_O::call_count,R"lambda(((this !) ))lambda",R"decl()decl")
17:32:17
drmeister
I hacked BrendanGregg's flamegraph.pl code to generate colors for Clasp. I'll have the prettiest flame graphs at the ball!
17:33:07
Bike
drmeister: okay, weird thing - the Derivable class in clbind has some code using _isgf, but as far as i can tell, it doesn't derive from FuncallableInstance_O, just Instance_O
17:33:13
drmeister
kpoeck: Yeah - distclean - I removed the slot and method for that and then added the slot back but not the method - sorry.
17:34:57
drmeister
https://github.com/clasp-developers/clasp/blob/dev/include/clasp/clbind/derivable.h#L87
17:35:20
Bike
the isgf offset is also exposed to the compiler for some reason, but that doesn't seem to be used so goodbye
17:40:05
Bike
also, get-funcallable-instance-function doesn't actually return a function, just a pointer. is that actually helpful?
17:42:05
Bike
that makes no sense with how it's used in compile.lsp, though, so that's already broken.
17:59:59
drmeister
Bike: Can you give me a heads up when you update sicl and the sicl hash in wscript and make these funcallable-instance-function stuff.
18:01:04
drmeister
I'll pull, rebuild and build some flame charts that look the the flame charts I posted a couple of days ago.
19:04:41
Bike
i thought of a way that i think would work to be able to "copy" a whole closure into a funcallable instance without needing locks. but it's kind of complicated so i'm just going to leave it in a comment.
19:27:12
Bike
bclasp closures work differently from cclasp ones... so my attempt doesn't work... i think i can figure this out though.
19:27:42
kpoeck
Bike: now that you fixed the thing that allows to compile-file unicode-strings and maintain them, what about https://github.com/clasp-developers/clasp/pull/717
19:28:32
drmeister
Bike - yeah - bclasp closures close over an Environment_O object don't they? But it's an object in one of the closure slots isn't it?
19:30:32
drmeister
There's some fields in the ClosureWithSlots_O C++ class. The closureType field tells you if it's a bclaspClosure and the 0th slot contains that environment.
19:30:54
drmeister
Now I'm puzzled why the length is zero - it should be one and that slot should contain NIL.
19:31:29
Bike
but i'm checking whether the length is zero. so it's basically false unless you put some non-fastgf function in.
19:39:01
Bike
of course if id just skipped to my general solution this wouldn't be necessary. let's see
19:49:03
kpoeck
Is this a message from clasp: Probably a BUG: slot SELFWARD-OPERATION in #<The STANDARD-CLASS ASDF/LISP-ACTION:COMPILE-OP> stopped existing between compile and load
19:49:13
kpoeck
Is this a message from clasp: Probably a BUG: slot SELFWARD-OPERATION in #<The STANDARD-CLASS ASDF/LISP-ACTION:COMPILE-OP> stopped existing between compile and load
19:52:51
Bike
drmeister: i am fairly sure it should be fine to call an instance while another thread is set-funcallable-instance-function-ing it. But calling s-f-i-f on the same instance in two threads simultaneously might be weird. can we mark the body a critical section or whatever?
19:53:21
kpoeck
compiling the file in https://gist.github.com/kpoeck/2b77d621b710f0bb8bc5dbd591905069
19:56:38
Bike
yeah, we should use clasp ffi for tests like that. ...anyway let me see if i can figure out the bug
19:59:07
stassats
since quicklisp is loaded twice, although i would not expect it to load asdf twice too
20:06:03
kpoeck
Well I can make that as a part of "Library tests" if you feel it is weird to do otherwise
20:15:11
selwyn
last time i tried to load trial, it complained about something not being a superclass. but i think you have an issue ticket about that already
20:28:17
kpoeck
Bike: are now all literal strings that are loaded in a compiled-file of type (SIMPLE-ARRAY ...) and no longer SIMPLE-BASE-STRING?
20:29:16
Bike
all i changed is that now strings that aren't base-strings aren't dumped as base-strings
20:29:31
kpoeck
core_float_to_string_free seems to assume that (ext:float-nan-string most-positive-double-float) returns a SIMPLE-BASE-STRING which is no longer true
20:32:01
Bike
oh, well, if you have your read "" as a character string patch, it'll not be a base string any more
20:44:07
kpoeck
In lispworks there is *DEFAULT-CHARACTER-ELEMENT-TYPE*, do we have something like that?
20:53:24
kpoeck
yes, if i set (setq *DEFAULT-CHARACTER-ELEMENT-TYPE* 'character) than (type-of "") returns SYSTEM:SIMPLE-AUGMENTED-STRING
20:54:21
kpoeck
(setq *DEFAULT-CHARACTER-ELEMENT-TYPE* 'base-char) (type-of "") -> SIMPLE-BASE-STRING
20:56:55
kpoeck
so with default-settings (setf (char (copy-seq "AAA") 0) (code-char 256)) fails in lispworks too
22:39:53
drmeister
./waf configure had problems upgrading sicl - so I removed it and then ./waf configure works.
23:23:21
stassats
as i said "if that's the quality of the lisp code what can be said about the whole benchmark?"
23:24:03
karlosz
thanks! that's good news then, because i have other patches that sped my build up more :)
23:24:05
stassats
but i guess it's too much to ask for well written lisp code, but in how many other languages are they not experts?
23:26:19
stassats
and it feels like the exercise in "make a wrong statement on the internet and wait for it to be corrected"
23:26:37
stassats
except for "publish poorly optimized code and wait for someone to prove you wrong"
23:41:31
selwyn
yeah i got that earlier. i thought it went away, but it turns out that i actually built the latest clasp with AST - i just realised
23:43:55
selwyn
could it be due to not having distclean'd? it's what i assumed when i saw it and indeed i hadn't
23:45:56
Bike
https://github.com/robert-strandh/SICL/blob/87eb4fead5c65eaa5b8954ca2f7fa47cf5dd08e8/Code/Cleavir/CST-to-AST/convert.lisp#L17
23:48:21
Bike
it matches the backtrace too, that method is the last function before the error basically
23:51:05
karlosz
this is why i stuck with cherry picking the patches i made - even loading sicl in clisp broke a couple of things recently
23:59:23
Bike
well i was going to make more cleavir changes tomorrow anyway, so i can update it with that
23:59:41
Bike
related, if anyone knows what name i should use for the multiple-value-setq-prog1 thing, now's the time