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

See whether we can materialize methods for clean closures as source #226

Open
theseion opened this issue Jan 6, 2018 · 2 comments
Open

Comments

@theseion
Copy link
Owner

theseion commented Jan 6, 2018

From the mailing list:

Hi guys,

Let me try to explain the current situation. Fuel does check if a closure is clean or not. If clean, it avoids serializing the whole stack. The way it does this is to simply clean the outerContext of the block to NOT have a sender. See methods CompiledMethod >> fuelAccept:
So..a clean closure would avoid serializing the sender and the sender of the sender and ... the whole stack. But... at the very least it does need to serialize the outerContext of that closure. That outerContext (even if the block is clean) has a reference to the method in which the closure was compiled. See this example:


^ [ :aNumber | Transcript show: aNumber ]'.

(TestCase new perform: #deleteMe) cleanCopy outerContext method.

Do you see how even a clean closure will still serialize ONE compiled method? (#deleteMe in this case). That is the method that could have changed bytecodes (remember that by default, Fuel does NOT fully serialize the methods if they are installed in classes...it will simply serialize the necessary information for looking it up at runtime) and that's where Fuel could bark even for clean closures. But of course, far less chances than serializing all the methods of the stack.

A more robust solution might be that if the block is clean, then do not serialize the closure as a closure with a clean context, but as a string (source). Then at materialization time, compile. The problem is that as far as I remember we never implemented substitution hook at materialization (only for serialization) so I don't have a place to hook. Yeah... you could do a become: with a postMaterialziationAction but that;s too hackish hahaha.

Hope this helps,

--- Searched for some few usages of SortedCollection and made sure, that
there is no sortBlock but only the default sort behaviour.

Can't we have Fuel handle the sort block if it's clean? I feel like we
discussed this before but can't recall the conclusion…

Fuel does handle blocks perfectly, as long as you don't upgrade the
underlying Pharo from version 1.4 to 6.1 (or similar big step), like I
did... The reason is the change of the bytecodes of a compiled method
somewhere between 1.4 and 6.1, which will then not materialize correctly
and raises a FLMethodChanged error. Therefore I had to deal with it,
like described before.

Cheers, Andreas

@theseion
Copy link
Owner Author

theseion commented Jan 6, 2018

This could be nice as it would probably mitigate the issues that crop up when a compiled method has changed it's bytecode. In many cases (e.g. when inspecting stack traces) we don't really care that the method has changed.

@stale
Copy link

stale bot commented May 18, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will remain open but will probably not come into focus. If you still think this should receive some attention, leave a comment. Thank you for your contributions.

@stale stale bot added the stale label May 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant