freenode/#sicl - IRC Chatlog
Search
17:29:13
karlosz
it's not that im particularly concerned about the speed, but if there are algorithmic improvements that on't complicate things too much ill take it
17:29:46
karlosz
in particular from profiling it seems that most of the time spent in eclector is now done reconstructing, and it also shows up a bunch in cst-to-ast, so any improvement helps
17:30:16
karlosz
i've gotten rid of C++ unwinding through generated code improvements, so it's easier to see what is really happening now with the reader
17:31:50
karlosz
right now clasp conses like on the order of 3 GB while compiling something small like asdf, so reducing consing by3% helps more than it seems
17:43:28
scymtym
karlosz: that is an unexpected result since Eclector should only call RECONSTRUCT for expression produced by reader macros
17:46:56
scymtym
RECONSTRUCT is called when the expression is not a proper list or does not correspond to the list of child CST results in the sense i mentioned above
17:52:39
scymtym
in the flamegraph, RECONSTRUCT is called from LEFT-PARENTHESIS (via some other functions). if that happens for a significant fraction of expressions, the source code is unusual or something is not working as intended (or RECONSTRUCT is so expensive that it shows up like that despite being called rarely)
18:04:58
Bike
maybe asdf is a weirder example because it's alle val-when? i guess not in a reader way though
18:08:22
scymtym
yes, RECONSTRUCT should only be invoked for things like (1 #.(list* 7 (second '(2 (3 4) 5))) 6) where for #.(list* 7 (second '(2 (3 4) 5))) the expression is (7 3 4) but child list is the the singleton list containing the CST for (list* 7 (second '(2 (3 4) 5)))
18:15:36
scymtym
maybe also worth special-casing to avoid RECONSTRUCT? i don't know how frequent those are
18:20:34
Bike
i tried tracing reconstruct. when compiling (defun foo () nil), it's called twice before cst-to-ast starts - once on DEFUN, and then once on (FOO NIL NIL)
18:21:02
Bike
(i have to go through compile-file since we don't use the cst reader for just READ calls. awkward)
18:22:18
scymtym
you could try tracing ECLECTOR.PARSE-RESULT:MAKE-EXPRESSION-RESULT that's where the call would happen in the cst reader
18:25:45
Bike
for the longer file i'm bashing my head on, it's called with expressions of :use, (:CL #:ALEXANDRIA), DEFPACKAGE, ("BUILDER-TEST" (:USE #:CL #:ALEXANDRIA)) for a defpackage form
18:30:44
scymtym
can you try whether (eclector.concrete-syntax-tree:read-from-string "(:use :cl #:alexandria)") calls RECONSTRUCT?
18:32:32
Bike
let me see... the "list structure with corresponding" elements code looks like what you said, but the commit...
18:34:09
Bike
https://github.com/clasp-developers/Eclector/blob/master/code/concrete-syntax-tree/read.lisp#L24-L47
18:38:11
Bike
yeah, looks like (list-length expression) is 2 (for (1 2)) but there are three children
18:38:23
karlosz
btw, i dont think it was clear but the flamegraph you both were looking at is actually from compiling src/lisp/kernel/lsp/predlib.lisp
18:41:34
scymtym
yes. but let me see whether i can suppress the parse result creation. that would be a bit better
18:47:54
karlosz
i'd like to keep clasp compilation unwindless if possible, and that means using lexical exits over signals
18:48:25
scymtym
but you could try s-expressionists/eclector/without-signal it looks like i rebased that in august and maybe accidentally fixed the issue along the way
18:48:50
karlosz
i guess that poses a dilemma since it seems like the signals based code is still currently significantly cleaner?
18:49:18
scymtym
sorry, the branch is s-expressionists/eclector/without-signal-and-avoid-return-from
18:50:05
scymtym
i also have much more confidence in the signal-based code since it avoids the in-band signaling issue. case in point being the problem bike found
18:53:31
karlosz
return-from is no longer an issue since we wrote optimizations to optimize that away, but in-band signalling seems to be a problem inherit in lexical exits
18:59:30
scymtym
and probably makes kpoeck happy since you'll also get the fix for uncommon spellings of #C
19:17:49
scymtym
karlosz: do you want to add the ASSERT call to the CST test or i just do it and push?
19:28:29
kpoeck
for the simple reason that it does not exist in https://github.com/clasp-developers/Eclector
19:35:17
scymtym
i thought master wouldn't work even with the sharpsign-single-quote issue resolved because signaling is still a problem?
19:52:29
kpoeck
scymtym could you add sharpsign-single-quote/relaxed so that can use master unchanged? I use master for clasp but added sharpsign-single-quote/relaxed locally
19:54:56
scymtym
kpoeck: i can do that but i thought using eclector/master makes your build slow because of the heavy use of signals in eclector/master
19:57:17
Bike
err are we? it's probably going to be kind of slow if there's actual unwinding regardless
19:59:31
Bike
i don't have any plan for making signal + handler-case fast, though. contification like karlosz is doing reduces unwinds when you return-from a local function into the head function, to oversimplify
20:01:08
Bike
SIGNAL control transfers are inherently dynamic so i'm not optimistic about optimizing much
20:01:54
karlosz
and maybe in the future we can find an elegant dynamic-signal-less algorithm that is an unconditional improvement
20:02:43
scymtym
yeah, i mean it is possible that the signal-less version is faster/better on other implementations, too
20:03:23
kpoeck
but right now we neither use without-signal-and-avoid-return-from nr newest eclector
20:28:32
scymtym
Bike: s-expressionist/eclector/without-signal is now rebased onto master and has the SHARPSIGN-SINGLE-QUOTE/RELAXED change as well as the "without signal" change. it does not avoid RETURN-FROM
20:29:30
scymtym
ACTION forgot to check that the original goal of eliminating bogus RECONSTRUCT calls is still achieved
20:35:06
kpoeck
so I will test : s-expressionist/eclector/without-signal and change wscript if it works