freenode/#clasp - IRC Chatlog
Search
15:04:10
kpoeck
and woudn't it be nice if (read-from-string (with-output-to-string (*standard-output*) (core::print-cxx-object (make-hash-table) *standard-output*))) worked?
15:08:13
kpoeck
That works #I(CORE:HASH-TABLE-EQL (CORE:REHASH-SIZE . 2.0) (core:REHASH-THRESHOLD . 0.7)(core:data . nil))
15:11:05
drmeister
#i(CHEM:MOLECULE (:CONTENTS . #A(COMMON-LISP:T (1) (#i(CHEM:RESIDUE (:CONTENTS . #A(COMMON-LISP:T (2) (#1=#i(CHEM:ATOM (:NAME . :C1) (:BONDS . #A(COMMON-LISP:T (1) (#2=#i(CHEM:BOND (:A2 . #3=#i(CHEM:ATOM (:NAME . :C2) (:BONDS . #A(COMMON-LISP:T (1) (#2#))) (:HYBRIDIZATION . :SP?) (:ELEMENT . :C))) (:A1 . #1#) (:ORDER . :SINGLE-BOND))))) (:HYBRIDIZATION . :SP?) (:ELEMENT . :C)) #3#))) (:NAME . :R) (:UNIQUE-LABEL)))))
15:11:35
kpoeck
as does (progn #1=2.0 #2=0.7 #I(CORE:HASH-TABLE-EQL (CORE:REHASH-SIZE . #1#) (core:REHASH-THRESHOLD . #2#) (core:data . nil)))
15:19:38
drmeister
Sure. Give me a sec - I'm putting a few changes into the void Bond_O::fields method...
15:40:37
Bike
so if we use the eclector reader with the eclector ##/#= reader macros on cxx objects, it can't possibly work, right?
15:41:11
drmeister
Soon - I'm going through my usual edit-compile-error-edit-compile-error cycle with C++.
15:41:32
drmeister
You probably spotted several compiler errors in the code above - let alone runtime errors (sigh).
15:43:25
Bike
https://github.com/clasp-developers/clasp/blob/dev/src/lisp/kernel/lsp/sharpmacros.lsp#L53-L56 so put this bit in a FIXUP method?
15:45:53
drmeister
Can you tell me how this works? There needs to be an eclector.reader:fixup method for cxx-objects?
15:46:18
Bike
I think uI figured FIXUP is basically eclector's equivalent to the circle-subst function i linked
15:46:51
Bike
when you read an object with ## in it, you create an actual object but stuff circularity markers in; they you iterate through the object with circle-subst or fixup replacing them.
15:47:42
drmeister
I see - so the circle-table argument in (make-record-patcher circle-table) needs to be dealt with somehow.
15:48:11
drmeister
This reader fixup stuff is like thermodynamics. I learn it and forget it over and over and over again.
15:48:20
Bike
i don't know how fixup works, but i imagine it can't be that much different from what clasp does itself
15:48:44
drmeister
It can't be. I'll start by looking at my make-record-patcher and what it needs...
15:50:51
drmeister
The Bonds_O::fields method does four different things depending on what the Record_sp argument is.
15:51:10
drmeister
It writes out data, reads in data, initializes the object and patches the fields in the object.
15:51:28
kpoeck
We need the equivalent of https://github.com/s-expressionists/Eclector/blob/wip-reader-implementation-documentation/code/reader/fixup.lisp#L28-#L32
15:51:43
drmeister
kpoeck: I'm not making any changes - I'm just trying to blow dust off my neurons.
15:53:22
drmeister
(ECLECTOR.READER:FIXUP #<ECLECTOR.READTABLE::CLASP-NON-CST-ELECTOR-CLIENT> #<MOLECULE :M 0> #<HASH-TABLE-EQ :COUNT 0 :SIZE 16 @0x12c94ba58> #<HASH-TABLE-EQ :COUNT 3 :SIZE 16 @0x12c94ba08>)
15:56:26
Bike
circle-table looks more like the seen table. you can tell, since circle-subst only puts in T as values.
15:57:18
drmeister
kpoeck: I use this everywhere. I think the class will be something very generic...
15:59:23
drmeister
We can't iterate over them. We have to use the Record_O facility and the 'fields' method.
16:00:06
drmeister
So we have to convert the arguments of eclector.reader:fixup into a circle-table and pass that to core:make-record-patcher.
16:01:24
drmeister
Here - I'll cook something up - but it will have a problem that I don't know how to solve at the moment...
16:04:46
Bike
Yeah. I think for the actual temp-to-final-value mapping we store the final value inside the temp itself, which is different from how Eclector maybe does it.
16:09:48
Bike
field() calls record_circle_subst, which calls core:circle-subst to get the value to patch with
16:10:48
drmeister
So the idea is that eclector carries around another hash table of mappings? And clasp sticks them where?
16:11:12
kpoeck
drmeister (mapcar #'first (core::encode (make-hash-table))) returns the list of slots
16:11:13
Bike
well, right now clasp doesn't have a way to get the new object from a hash table. it interleaves these steps.
16:12:07
Bike
we could define this in a more general way - have a field() method that calls some function on each field value, then puts the return value from that function in as the new value.
16:12:19
Bike
and then have eclector pass it a closure that looks things up in the hash table or whatever.
16:13:11
Bike
kpoeck: we need to modify an existing object rather than create a new one (ofc) and right now i think the closest thing to doing that generally is this fields() interface. there's no slot-value
16:14:16
drmeister
Bike: Do you see where clasp stores the mapping from the placeholder to the final value? I'm not seeing/recalling it yet.
16:14:53
Bike
https://github.com/clasp-developers/clasp/blob/dev/src/lisp/kernel/lsp/sharpmacros.lsp#L94
16:15:46
Bike
that's where the final value is set, and it's read in circle-subst here https://github.com/clasp-developers/clasp/blob/dev/src/lisp/kernel/lsp/sharpmacros.lsp#L13-L15
16:19:05
Bike
You can see how it works in fixup-place in the file kpoeck linked https://github.com/s-expressionists/Eclector/blob/wip-reader-implementation-documentation/code/reader/fixup.lisp#L28-#L32
16:19:18
kpoeck
(defmethod fixup (client (object core::cxx-object) seen-objects mapping) (loop for name in (mapcar #'first (core::encode object)) when (slot-boundp object name) do (fixup-place (slot-value object name))))
16:20:19
Bike
I don't think slot-value on a cxx-object is going to work, but maybe we can get an alist with encode, modify the alist with the new values, and then write the alist back into the object with decode.
16:22:26
Bike
so we can pass decode an alist of (slot-name . slot-value) and it'll set all the fields? that should be enough.
16:23:30
drmeister
https://github.com/clasp-developers/clasp/blob/dev/include/clasp/core/record.h#L127
16:24:28
Bike
yeah, that's what i was referring to earlier. it calls record_circle_subst which calls circle-subst. that won't work with eclector.
16:25:47
Bike
if you figure out a way to get slot-value and (setf slot-value) and class-slots equivalents instead, there will be no need for that
16:25:57
Bike
but we should have something that works at all before concerning ourselves with memory usage
16:26:12
drmeister
That's not possible. I'm thinking about modifying the fields/field/Record_O mechanism
16:26:47
Bike
actually, why isn't it possible? seems like all the information is there in one way or another
16:27:01
drmeister
Ok, I wont' say not possible. It would actually be quite beneficial to get access to C++ slots.
16:27:29
Bike
but for now we should just try the encode-decode thing so nobody has to rewrite anything
16:27:57
drmeister
We have the table we generate for MPS. It provides offset and type information about Clasp slots.
16:30:33
kpoeck
(let ((obj (make-hash-table))) (values (core:encode obj) (core:encode (core:decode obj '((CORE:REHASH-SIZE . 3.0) (core:REHASH-THRESHOLD . 0.5) (core:data . nil))))))
16:33:25
Bike
i don't think the hash table code uses encode or decode anywhere, but it's a convenient example class to work with
16:35:18
Bike
if you want cando you can just git clone it and symlink the directory from clasp/extensions
16:35:38
Bike
all the C++ code is available in iclasp even if you don't load all the cando lisp stuff
16:38:40
drmeister
(1) git clone https://github.com/clasp-developers/clasp.git cando (2) cd cando; git checkout dev (3) cd extensions (3) git clone https://github.com/cando-developers/cando.git (4) cd cando; git checkout dev
16:40:17
drmeister
The (something-something-something object-encoded seen-objects mapping) ... what should that look like?
16:40:52
Bike
drmeister, seriously, i think kpoeck can handle this, you don't need to write it yourself. kpoeck knows eclector better than you or i do
16:48:18
kpoeck
drmeister I meant something like in the comment to https://gist.github.com/drmeister/44c864a2cfc959ee404bfc17ab60991a
16:50:06
drmeister
Yes - for this part - I agree - let's get it working with encode/decode . But I'd like to understand what is going on. But please proceed.
16:53:42
kpoeck
What I was trying to do is the equivalent of https://github.com/s-expressionists/Eclector/blob/wip-reader-implementation-documentation/code/reader/fixup.lisp#L28-#L32
16:54:32
drmeister
There are almost no 'fields' methods in clasp - so I don't know how to put together an example outside of cando - so... kpoeck it's good you have cando building.
16:55:02
drmeister
kpoeck: Cando adds about 150 C++ files and lots more C++ classes/functions/methods.
16:55:41
kpoeck
but going on, once we have the class, we need to iterate over the slots&values of the instance to eventually patch up the slots
16:55:45
drmeister
Yeah but core::cxx-objects is just the base class - it doesn't implement ::fields itself. It's up to each class to implement its own fields method.
16:56:50
drmeister
We can't iterate over the slots&values of C++ instances - they are too low level to do that. You can iterate over the entries of the alist returned by (core:encode object) and then you can write that rewritten alist using (core:decode object new-alist)
16:57:38
kpoeck
Could you look at my comment on https://gist.github.com/drmeister/44c864a2cfc959ee404bfc17ab60991a
16:58:52
drmeister
Ah - I have to click on it. icloud shows the first lines and I thought that was all there was.
17:01:03
Bike
drmeister: if you want to do it without consing, what you could do is expose a generalized patcher kind of C++ method, that iterates over the slots (in whatever sense), calls a given lisp function on each value, takes the return value from that function, and puts it in the slot
17:04:42
drmeister
Right (1) get the alist using (core:encode object) (2) fix up all the values with rplacd (3) stuff the values back into the object with (core:decode object fixed-up-alist)
17:05:45
kpoeck
Perhaps the recursion for (fixup client value-fixed seen-objects mapping) is missing
17:07:38
Bike
(rplacd slot-and-value (if found-p (progn (setf fixed-up t) value) (fixup client value-old ...))) or something
17:08:52
kpoeck
did a new version n https://gist.github.com/drmeister/44c864a2cfc959ee404bfc17ab60991a
17:10:22
drmeister
There's a typo value-fixed is never defined? Should it be 'value'? or rename 'value' to 'value-fixed'
17:12:46
drmeister
Also, if fixed-up isn't set to T (none of the alists values needed fixing up directly) but the recursive (fixup ...) did fix something up. Then we still need to call (core:decode ...)
17:12:52
Bike
in the analogous standard-object method it uses current-value, which is the value passed to gethash
17:13:27
Bike
like, the cxx object will have a cons in it, and fixup will modify the cons, but the cxx object just refers to the same cons
17:13:53
drmeister
So we only call (core:decode object new-alist) if the alist in the loop is modified.
17:15:18
drmeister
No worries kpoeck. This is difficult stuff and we are all working towards the correct solution.
17:16:58
kpoeck
Updated version in https://gist.github.com/drmeister/44c864a2cfc959ee404bfc17ab60991a
17:17:11
drmeister
I spend plenty of time working by myself with just my demons. It's always a joy to solve a problem with you guys.
17:19:23
drmeister
Ok. Now isn't there something where if the (not (eq value value-old)) then we know that there was no substitution. Could this be more efficient if we were using this property of value/value-old?
17:19:58
Bike
Don't we just know whether there's a substitution from whether there's something in the hash table?
17:21:11
drmeister
Bike: I agree - it may be all we need to know that there was something in the hash-table - that found-p is T.
17:21:58
drmeister
Ok, on running fixup on 'value' - does that happen somewhere else? Or do we have to do it here?
17:22:40
Bike
Yes, we have to do it here, because every component of every compound object needs to have fixup run on it once.
17:23:47
kpoeck
In general fixup is called from here: https://github.com/s-expressionists/Eclector/blob/wip-reader-implementation-documentation/code/reader/read.lisp#L28
17:23:55
Bike
if you're worried about efficiency i already told you what we could do to avoid the consing of encode
17:24:36
kpoeck
drmeister does this change - although perhasp consing to much - helping the error you had?
17:26:14
scymtym
Bike: i think the complete "seen" table is unavoidable since you could stick #.*my-pre-existing-circular-object* into the input
17:27:08
drmeister
Bike: your suggestion to reduce consing was... "drmeister: if you want to do it without consing, what you could do is expose a generalized patcher kind of C++ method, that iterates over the slots (in whatever sense), calls a given lisp function on each value, takes the return value from that function, and puts it in the slot"
17:28:48
scymtym
Bike: i was referring to the necessity of putting every object in the SEEN-OBJECTS table in FIXUP
17:30:35
drmeister
I think a place to put this would be... src/lisp/kernel/cleavir/activate-clasp-readtables-for-eclector
17:33:09
drmeister
Bike: To follow your suggestion I could add a new mode to Record_O called 'eclector_patching' - then I would add the callback to Record_O and modify all code that implements 'case patching:' to add 'case eclector_patching:' and that code would do what you say.
17:33:39
drmeister
There would be other effects though - we might need to rewrite the other clasp specific eclector.reader:fixup methods for hash-table for instance.
17:34:48
Bike
eclector has a built in fixup method for hash tables, right? that just uses gethash and (setf gethash)?
17:35:55
drmeister
Maybe not - I'm tossing that out there and then you challenge me and then I dig deeper.
17:36:23
Bike
i mean i dont understand what you're thinking? "rewrite the other clasp specific eclector.reader:fixup methods"? but we have no specific methods
17:37:00
drmeister
Clasp has this: https://github.com/clasp-developers/clasp/blob/dev/src/core/hashTable.cc#L602
17:37:35
drmeister
Ok, good. It's not even consistent. https://github.com/clasp-developers/clasp/blob/dev/src/core/hashTable.cc#L628
17:37:35
Bike
and as you can see, we're not even using it for clasp's native ##, since the patching: case just crashes clasp
17:38:04
drmeister
https://github.com/clasp-developers/clasp/blob/dev/src/lisp/kernel/lsp/sharpmacros.lsp#L32
17:39:48
kpoeck
I'd say teh coed you just pasted is no longer used after bootstrapping and we handle it here
17:39:51
kpoeck
https://github.com/s-expressionists/Eclector/blob/wip-reader-implementation-documentation/code/reader/fixup.lisp#L34-#L41
17:44:59
drmeister
I'll check if the method is making any difference by restarting cando and trying it without it.
17:46:26
drmeister
Ok, it's having some effect. I'm seeing something different. Hang on - I'll make it print out more info.
17:51:19
drmeister
The atom in the bond in C1 is being fixed up properly. But the bond object in C1 should also be used to fixup the bond object in C2 - that is not happening.
18:06:05
drmeister
Let's take it slow. Here is a new change I made to the method to get more insight...
18:08:04
drmeister
It doesn't find a fixup - of course not - it's a vector containing (T) and this is all that is in the mapping...
18:09:46
drmeister
https://github.com/cando-developers/cando/blob/dev/include/cando/chem/atom.h#L202
18:10:05
drmeister
VectorBond... https://github.com/cando-developers/cando/blob/dev/include/cando/chem/atom.h#L167
18:10:49
drmeister
https://github.com/clasp-developers/clasp/blob/dev/include/clasp/core/record.h#L139
18:11:20
drmeister
So Clasp's Record_O feature needs to know what is going on. encode/decode can't work without support from Record_O.
18:12:29
drmeister
Ok. But fixup is going to have to tell the caller that it fixed something up - correct?
18:13:18
Bike
the problem is that fixup will alter this constructed lisp vector, but the vector isn't actually what's in the cxx object.
18:14:24
Bike
it means the encode/decode won't actually work, since clasp isn't actually able to represent the slot value as a lisp object.
18:15:15
drmeister
What I need to do is change how Record_O works. I need to make it so that it can use what Eclector passes to eclector.reader:fixup
18:15:58
Bike
well, it could still work with what i said with the one-arg function, it would just have to be more involved
18:16:19
drmeister
Damn, I couldn't put my finger on what was wrong - but my subconscious has been screaming at me for the past couple of hours.
18:17:24
drmeister
No - I don't think we can avoid it. I have data structures that aren't exposable to Common Lisp.
18:18:53
drmeister
kpoeck: The problem is I have data structures in the C++ code that aren't/can't-be exposed to Common Lisp.
18:19:14
drmeister
I could get rid of them - and use only Common Lisp data structures - but that would require some work.
18:19:48
drmeister
Like - in this case - a SimpleVector_O containing Bond_sp could work without too much trouble.
18:20:18
drmeister
But I'd have to initialize it in a different way that I do now and we would have to change the code that touches the Atom_O::bond slot
18:24:11
Bike
i don't think you should change how all the classes are laid out. besides being a lot of work, it would be nice for this all to work with a lot of different C++ code.
18:24:49
drmeister
I can change the 'patching' mode so that it doesn't call record_circle_subst directly, but it calls a closure.
18:26:48
drmeister
What if we have Record_O take a function in patching mode and that function takes two arguments - a 'mapping' and the object and it returns the object or another object from the mapping?
18:29:44
drmeister
https://github.com/clasp-developers/clasp/blob/dev/include/clasp/core/record.h#L171
18:35:15
drmeister
core:make-record-patcher adds an optional argument for the callback and the default value is a trampoline to record_circle_subst
18:45:49
drmeister
Maybe - I'll give it a try with this and we will see. The Record_O::fieldxxx template functions and core:circle-subst handle recursion properly.
18:48:49
drmeister
That calls core:circle-subst. I'd need to substitute a different function. What would that different function do?
18:50:16
kpoeck
Could you please also have a look at https://gist.github.com/kpoeck/d1b2df2d670dc42e1cdc3f4e8144682b and see whether this looks fine?
18:50:53
kpoeck
I meant the result in https://gist.github.com/kpoeck/d1b2df2d670dc42e1cdc3f4e8144682b
18:51:52
drmeister
kpoeck: The problem is going to be in the C++ - the field on line 3 will not work properly.
18:53:15
drmeister
A Vec0<Bond_sp> is not representable in Common Lisp - so a simple-vector is created to represent it. But they aren't identical objects so the fixup will fail.
19:28:39
Bike
having the record patch just take a closure lets you do literally any operation at all, so i don't see the point of having a mapping
21:18:05
paulapatience[m]
Ok, so I'm trying to build a debug version of clasp with ./waf build_cboehm_d, but I get a compilation error. (I didn't distclean the directory since my previous (successful) compilation of nondebug clasp; should I have?)
21:18:05
paulapatience[m]
My changes to wscript, my wscript.config, and the compilation log (with -v): http://ix.io/2gFM
21:20:36
paulapatience[m]
Ok I figured the undefined functions might be the problem, but it's true that no other error is mentioned
21:22:02
paulapatience[m]
Perhaps I'll try compiling from a clean checkout then. But I am right in assuming that ./waf build_cboehm_d builds the debug version?
21:24:01
drmeister
./waf build_cboehm_d does build the debug version. It's quite slow and I hardly use it because it is so slow.
21:24:16
paulapatience[m]
(I don't have the lldb/assembly chops to debug the -O3 optimized release version)
21:25:57
paulapatience[m]
I'm going to have to go to the store for some groceries but I'll be back in a bit
21:43:26
drmeister
It's not even necessary - it's lazy initializers failing that were initialized when clasp started.
22:06:34
paulapatience[m]
So it would be a Linux-only problem? (though I seem to recall kpoeck having the same problem on macOS)
22:58:18
drmeister
I'm not sure. On linux it's a static constructor that shouldn't be needed but is being invoked because I'm including clasp headers that declare it.
22:58:57
drmeister
Static constructors are getting a big refresh by llvm11 - but that's not ready yet.
22:59:16
drmeister
Another idea would be to compile and link the code with Clasp and load the dynamic library.
23:00:45
paulapatience[m]
Compile and link the cxx code with clasp rather than clang, you mean? Clasp can do that?
23:03:44
drmeister
Right now we are taking a short-cut. We are compiling the demo code to bitcode. The .bc file in each directory
23:05:25
drmeister
Here we are kind of spoofing that. We are compiling a C++ file to bitcode and making it look like a clasp generated bitcode file. But it's not a perfect spoof because we compile C++ header files that are declaring static initializers.
23:06:19
drmeister
When we load the bitcode into clasp it compiles the bitcode to native code and links it against symbols that are already loaded.
23:07:02
drmeister
Maybe it's the code model. Maybe the symbols are more than 2GB away and they can't be linked?
23:07:36
drmeister
The linux JIT linker is being rewritten as we speak. By llvm11 this will be a lot easier.
23:07:54
drmeister
They are making the JIT linker work very much like the system linker. Right now everything is hacked together.
23:08:26
drmeister
That's my understanding. But it's informed by long discussions with Lang Hames - the developer of the new ORC JIT.
23:13:04
paulapatience[m]
ACTION sent a long message: < https://matrix.org/_matrix/media/r0/download/matrix.org/umdQtPUqjIZDjoLWKCOVkbUd >
23:16:08
paulapatience[m]
what command did you run to get that output? I can't seem to get the disassembly around frame #1
23:18:59
drmeister
It's putting 0x0 in %rax and then calling it two instructions later - that's crazy. So it makes me think the $0x0 should be an address that failed to link.
23:20:20
paulapatience[m]
I naively assumed that since we compile clasp with llvm we should debug it with lldb
23:20:28
drmeister
There's nothing wrong with calling 0x0 - until you get there and hit a page fault.
23:21:01
drmeister
Sure - but lldb needs to be installed on linux and I often don't take the trouble.
23:23:09
paulapatience[m]
I used to use gdb a lot when debugging microcontrollers a few years ago, but it's the kind of stuff you forget from lack of use...
23:25:33
drmeister
That's why I think the problem is linking. The linker should be putting an address in there.
23:35:49
drmeister
call void @_ZN3reg7type_idC2ERKSt9type_info(%"class.reg::type_id"* %1, %"class.std::type_info"* dereferenceable(16) bitcast (i8** @_ZTIN4core6Real_OE to %"class.std::type_info"*))
23:35:57
drmeister
Followed by %5 = call i64 @_ZN3reg17allocate_class_idERKNS_7type_idE(%"class.reg::type_id"* dereferenceable(8) %1)
23:40:43
paulapatience[m]
Ok those are the functions that we should be calling but that are called as 0 instead?
23:42:11
drmeister
There are only two calls, the first one seems to be to a reasonable address, the second one to 0x0
23:47:46
drmeister
Here's something interesting. When I compile it to an object file. A bunch of __cxx_global_var_init.xxx functions don't appear to generate code.
23:56:21
paulapatience[m]
I'm getting 0s for a bunch of other functions too, though. Is that normal?
0:01:49
paulapatience[m]
but the cxx_global_var_init ones are in the text section so I would imagine they don't get overridden. but i do not have much experience in this.
0:02:39
drmeister
I disassemble the startup function and it's full of 0x0 - it isn't linking at all.
0:06:41
drmeister
Lines 9, 14, 54 - You can search for $0x0,%rax and then see a call to *$rax right after that.
0:21:22
drmeister
I think the linking on linux is going to be broken until the JITLinker is completed for linux in llvm11
0:22:10
drmeister
I had this stuff working on linux at one point - but it's the hardest stuff to keep working - even though it's a signature feature of clasp.
0:23:42
drmeister
When we load bitcode - it needs to link the addresses in the native code generated from the bitcode to symbols in the executable - yes.
0:24:09
drmeister
We could use the system linker - but then we need to generate a clasp dynamic library to link against - we don't have that. We generate a clasp executable.
0:51:46
paulapatience[m]
You mentioned that the static initializers are already initialized when we call runStaticConstructorsDestructors (if I understood correctly). Would it be possible to disable the reinitialization somehow?
1:28:15
drmeister
No - because our static initializer is in the list of static initializers. It's just that we picked up a bunch more from header files.
2:31:12
Bike
fixup is called for effect, so it just needs to call fixup on value-old before returning it