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

Add compact attribute for hot classes #289

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gavr123456789
Copy link
Contributor

Vala lost 10 seconds, which at first surprised me, then I looked in db679b0 and after experiments, I found that the reason was private fields. I created an issue https://gitlab.gnome.org/GNOME/vala/-/issues/1116.
The maintainer of the language recommends the use of a Compact classes in high loaded code. Also Vala supports inline modifier, so I added it on hot methods too.

@nuald
Copy link
Collaborator

nuald commented Dec 9, 2020

Sorry, but micro-optimizations are out of scope of this project. You could see that other implementation are not optimized too, but rather represent the idiomatic code (as we don't compare algorithm implementations, but rather compilers/interpretators operating the similar algorithms). It's fine to submit another test that has optimizations though.

Please note that other implementations (including this one) may contain some non-idiomatic code and some code optimizations, we try to fix those discrepancies, but could miss something.

@gavr123456789
Copy link
Contributor Author

Doesn't this mean that you should disable JIT compiler warm-up?

@kostya
Copy link
Owner

kostya commented Dec 9, 2020

I am not really againts adding inline/compact for vala or for others (because it can be considered as also idiomatic way for speedup programs, not hacks). Like java for example using final word. But this public/private change looks strange for me as for user.

@gavr123456789
Copy link
Contributor Author

gavr123456789 commented Dec 9, 2020

Compact classes doesnt support private modifier yet. There will be compile error private fields are not supported in compact classes, so it can be clear to the user.

@nuald
Copy link
Collaborator

nuald commented Dec 9, 2020

@gavr123456789 We don't have JIT compiler warm-ups anymore, if you've noticed some, please let me know.

@kostya
Copy link
Owner

kostya commented Dec 9, 2020

btw, i think that disable index checks (that nim -d:danger doing), is much more hack, than inline.

@gavr123456789
Copy link
Contributor Author

gavr123456789 commented Dec 9, 2020

I have nothing to add except that I agree with @kostya. This is not changing the algorithm to make it work faster, it is a common thing that vala programmers will do if they write an algorithm rather than a GUI, but this may not be obvious to C# programmer.

@gavr123456789
Copy link
Contributor Author

So the discussion is over?

@nuald
Copy link
Collaborator

nuald commented Dec 12, 2020

@gavr123456789 Compact classes do not look like idiomatic data type (considering their limitations), and looks like are used for the use-cases there specific memory management is required:

Compact classes, so called because they use less memory per instance, are the least featured of all class types. They are not registered with the GType system and do not support reference counting, virtual methods, or private fields. They do support unmanaged properties. Such classes are very fast to instantiate but not massively useful except when dealing with existing libraries.

Using them for the performance tuning does look like a hack for me. That would be like using structs and global functions to operate them in OOP way in C++ - while it make sense in some use-cases, it's not an idiomatic type of working with classes in C++.

@kostya final keywork in Java is not necessary a performance optimization, but rather a safety measure. As for the inline - we don't have any code with inlines anymore (and good compilers do inlining automatically anyway). As for the -d:danger - agreed, but we don't have any limits on compiler options. If we don't want to allow it, we need to change README to include that criterion (I can take care of it if you want).

@kostya
Copy link
Owner

kostya commented Dec 12, 2020

I not sure about final, but seems it used for internal optimizations also. About compile keys i think we should use, only keys which would use in production, i doubt that -d:danger or -mbranches-within-32B-boundaries or -fbounds-check=off would be used.

@nuald
Copy link
Collaborator

nuald commented Dec 14, 2020

@kostya Added #290 issue to address it.

As for final, there are a lot of discussions about it, and actually using final could even decrease performance (as final creates new variable in memory, but reusing the same variable could be faster as it could minimize GC involvement). A lot of useful links are provided in https://stackoverflow.com/questions/30524582/java-final-variables-and-performance (including the duplicated answer). Anyway, using final is not caused by any performance reasons, but rather by the standard Java coding style (checkstyle utility and many Java coding styles recommend to use final variables and classes).

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

3 participants