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

Would it make sense to put all used selectors in the literal array? #5634

Closed
MarcusDenker opened this issue Feb 5, 2020 · 7 comments
Closed

Would it make sense to put all used selectors in the literal array? #5634

MarcusDenker opened this issue Feb 5, 2020 · 7 comments

Comments

@MarcusDenker
Copy link
Member

@MarcusDenker MarcusDenker commented Feb 5, 2020

Right now we do not put all the selectors in the literals that are in "Smalltalk specialObjectsArray at: 24":

"#(#+ 1 #- 1 #< 1 #> 1 #'<=' 1 #'>=' 1 #= 1 #'~=' 1 #* 1 #/ 1 #'\' 1 #@ 1 #bitShift: 1 #'//' 1 #bitAnd: 1 #bitOr: 1 #at: 1 #at:put: 2 #size 0 #next 0 #nextPut: 1 #atEnd 0 #'==' 1 #class 0 #'~~' 1 #value 0 #value: 1 #do: 1 #new 0 #new: 1 #x 0 #y 0)"

This is done to save space. Which is understandable for a System designed in 1978, it even makes sense in 2005. But today?

I propose we go to the bank and sell some "Mores law" stock and invest it in simplicity: add all these selectors when we compile method to the literal frame, if they are send in the method.

Positive:

  • for "senders of" We do not need to check for #specialSelectorIndexOrNil: and then do not need to scan the bytecode for the special selectors, speeding up all cases a little and "senders of" for all the special selectors a lot.
    Thus: system will get much simpler and faster

  • We had people getting very confused that "senders of" #== shows lots of false positives. The reason is that #ifNil: compiles to it on the bytecode level. If we would not query the bytecode for #==, none of these false positives would show. #3347
    Thus: We fix a real problem

Negative:
The only negative side is that the literal arrays will be larger for many methods. This could be a problem if the number of literals is used as a "code complexity" metric. I remember the JIT at some point doing something like that?

@pavel-krivanek

This comment has been minimized.

Copy link
Collaborator

@pavel-krivanek pavel-krivanek commented Feb 5, 2020

VisualWorks still uses the same optimization. However, I fully agree that we should simplify this.

@Ducasse

This comment has been minimized.

Copy link
Member

@Ducasse Ducasse commented Feb 5, 2020

How much memory are we talking?
Could we make this only for large images?

@MarcusDenker

This comment has been minimized.

Copy link
Member Author

@MarcusDenker MarcusDenker commented Feb 6, 2020

Doing only for large images makes no sense: the most important aspect is simplicity, making that conditional is exactly the opposite.

(And it would not speed up large images just if you look for senders of one of the special selectors)

@guillep

This comment has been minimized.

Copy link
Member

@guillep guillep commented Feb 6, 2020

I wouldn't do anything unless we add tests about this.

  • for "senders of" We do not need to check for #specialSelectorIndexOrNil: and then do not need to scan the bytecode for the special selectors, speeding up all cases a little and "senders of" for all the special selectors a lot.
    Thus: system will get much simpler and faster

Is this speed really relevant? I don't think so... Do you have measurements?

  • We had people getting very confused that "senders of" #== shows lots of false positives. The reason is that #ifNil: compiles to it on the bytecode level. If we would not query the bytecode for #==, none of these false positives would show. #3347
    Thus: We fix a real problem

I have the feeling that depending on the literal frame of methods is brittle and breaks always at the end. Particularly, how does this work with bytecode rewrites where the original sourcecode
shows something but behind something completely different is compiled? The ifNil: is just a simple but common case.

  • What happens with metalinks? If I use a metalink does it wrongly appear as a sender?
  • uFFI does method rewriting too => so it shows false positives too
  • my string interpolation implementation internally compiles as a send of #format: so we have a false positive here too.
@MarcusDenker

This comment has been minimized.

Copy link
Member Author

@MarcusDenker MarcusDenker commented Feb 6, 2020

Yes, you are right that it would just be a small improvement and not solve to problem for real. What we need (as you imply) is to make the distinction of "Language Model" and "Execution Model" of methods explicit: Maybe we need "Method" and "CompiledMethod", in some way... ?

  • CompiledMethod would be the one that is optimised for execution (and all space saving tricks allowed), they would not need to know their class and selector, for example (as in ST80).

  • Method would model the user's view as much as possible. It could be un-optimized bytecode or even something else.

The VM would deal with CompiledMethod, the IDE with Method. MetaLinks would change CompiledMethod, but not Method, and so on.

@guillep

This comment has been minimized.

Copy link
Member

@guillep guillep commented Feb 6, 2020

Yes, you are right that it would just be a small improvement and not solve to problem for real.

Don't take me wrong. What I mean is that I'd like to backup assertions like "the system will get faster" with a measurement

And avoid doing such a change (that will maybe impact people using senders :)) without a proper test.

What we need (as you imply) is to make the distinction of "Language Model" and "Execution Model" of methods explicit: Maybe we need "Method" and "CompiledMethod", in some way... ?

  • CompiledMethod would be the one that is optimised for execution (and all space saving tricks allowed), they would not need to know their class and selector, for example (as in ST80).
  • Method would model the user's view as much as possible. It could be un-optimized bytecode or even something else.

The VM would deal with CompiledMethod, the IDE with Method. MetaLinks would change CompiledMethod, but not Method, and so on.

We agree :)

@MarcusDenker

This comment has been minimized.

Copy link
Member Author

@MarcusDenker MarcusDenker commented Feb 19, 2020

I close this for now, next step is to experiment a bit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.