Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store copies of data at trace-time. #456

Closed
wants to merge 7 commits into from
Closed

Conversation

phmarek
Copy link
Contributor

@phmarek phmarek commented Jul 11, 2018

When passing structures or other identity-preserving items (arrays,
list heads) around, tracing the call chain wouldn't show how these
items evolve.

At least for the simple well-known cases we can store a copy of the
arguments; the time/space effort only occurs during tracing anyway
and is well spent, IMO.

@phmarek
Copy link
Contributor Author

phmarek commented Jul 11, 2018

This is basically a plea for discussion; I think it's worth doing.

@luismbo
Copy link
Member

luismbo commented Jul 11, 2018

@phmarek the usual way to achieve that is to store the print representation. (That's of course how purely textual traces work, but Lispworks does that too, IIRC.) That way, it'll be time/space-bound by *print-length* and *print-depth*. Also, for instance, in the structure case, the print representation would be more informative and accurate than a shallow copy.

Another idea would be tuse swank-presentation-streams (which would need to be fixed beforehand, since it's currently buggy). This way would get a printed representation that is inspectable. Upon inspection it should usually be clear if the object changed since being printed.

Yet another idea would be to do a shallow copy (as you've implemented) and use that to somehow point out slots that have been changed. At this point, I worry about the performance implications of this approach though. Perhaps, something like this can also be achieved by comparing printed representations, without or without presentations, and doing some sort of diff between the two. That'd be neat!

What do you think of these ideas?

@joaotavora
Copy link
Contributor

Another idea would be to fix then use swank-presentation-streams. This way would get a printed representation that is inspectable. Upon inspection it should usually be clear if the object changed since being printed.

This is how it should work (and is akin to very quickly refreshing the buffer). Though I don't think you need to mess with the swank-presentation-streams goo, the print representation is already inspectable (at least that's how I remember implementing it).

My two cents to add to Luis:

  1. If copying is done this has potentially huge performance implications and you should make it optional;
  2. Any copying strategy would break identity semantics that the user may or may not be counting on.

@phmarek
Copy link
Contributor Author

phmarek commented Jul 11, 2018

I tried with a PRINC-TO-STRING. What I didn't like about that was that it messed up the type information, as "4" might have been a single-character string, an integer, or perhaps even a symbol.

Furthermore, seeing different print formats (decimal, hex, octal, binary) of integers is really helpful, too; so I searched for a way to keep the representation as identically as possible.

@joaotavora: yeah, object identity gets lost with any other representation. Perhaps we should store the original argument as well, and then have some output like #1=... and #1# but #s<field: value>?

Still, I believe that (shallowly) copying STRUCTURE-OBJECTS is the right thing to do; it keeps most of the information, and should be fast enough.

@joaotavora
Copy link
Contributor

I tried with a PRINC-TO-STRING.

You probably want something like with-output-to-string and prin1:

(with-output-to-string (s) (prin1 "3" s)) ; => "\"3\""
(with-output-to-string (s) (prin1 3 s)) ; => "3"

Furthermore, seeing different print formats (decimal, hex, octal, binary) of integers is really helpful, too; so I searched for a way to keep the representation as identically as possible.

Indeed but there are ways to solve this using a smarter printer. SLY by default presents integers in various formats (and you can add your own).

should be fast enough.

Sound like famous last words. It really depends if the function that you're tracing is very deep/tight in your program's the critical path. You can probably wreck your process easily by tracing say cons accidentally.

@phmarek
Copy link
Contributor Author

phmarek commented Jul 11, 2018

should be fast enough.
Sound like famous last words. It really depends if the function that you're tracing is very deep/tight in your program's the critical path. You can probably wreck your process easily by tracing say cons accidentally.

Yeah, everything can be broken.

But tracing is not a permanent change ("just" reload the image ;), and tracing alone will already be expensive.

@joaotavora
Copy link
Contributor

joaotavora commented Jul 11, 2018

("just" reload the image ;),

Those are extremely large quotation marks

tracing alone will already be expensive

But now you want to make the price grow with the size of the objects being traced, which wasn't true before.

@phmarek
Copy link
Contributor Author

phmarek commented Jul 11, 2018

Well, how about having a few ways selectable via a special variable? *TRACE-STORE* as :STORE-VALUE, :STORE-PRIN1, and :STORE-SHALLOW-COPY with :STORE-VALUE as default (to be backwards-compatible)?

@joaotavora
Copy link
Contributor

Yeah, making it optional is probably a good idea. If like your implementation, I'll probably backport it to Sly.

@luismbo
Copy link
Member

luismbo commented Jul 11, 2018

This is how it should work (and is akin to very quickly refreshing the buffer). Though I don't think you need to mess with the swank-presentation-streams goo, the print representation is already inspectable (at least that's how I remember implementing it).

You're right. I was thinking that it'd be useful to inspect the object's sub-components directly, which is what swank-presentation-streams let's you do, but in retrospect it seems quite orthogonal to the topic at hand.

Any copying strategy would break identity semantics that the user may or may not be counting on.

Yep, another good point against copying objects.

Due to the asynchronicity of multithreaded connections we have
to use autodetection: by the time the "switch to json" command
has been processed the READ-MESSAGE loop has already been
invoked again, and so passing the decoder in as argument would
just give the old (stale) value again.

Other ideas discussed on #slime.
Copy link
Contributor

@easye easye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that's the right interface, but it returns a Java object https://docs.oracle.com/javase/8/docs/api/java/net/ServerSocket.html#getLocalSocketAddress-- which needs to be interpolated into whatever the defimplementation expects across the implementations.

@phmarek
Copy link
Contributor Author

phmarek commented Jan 21, 2021

shutting off, branch is used for other things ;/

@phmarek phmarek closed this Jan 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants