ov.h fast_elem_insert

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

ov.h fast_elem_insert

gareins
Hello!

in ov.h inside interpreter, why is fast_elem_insert implemented as a virtual method - by my knowledge no class inherits octave_value. This also creates a virtual table for octave_value and makes octave_value twice the size (instead of the size of one pointer, it is the size of two pointers).

Ps. this is my first time writing to a technical mailing list, so please forgive any wrongdoing.
Reply | Threaded
Open this post in threaded view
|

Re: ov.h fast_elem_insert

siko1056
O┼żbolt Menegatti wrote
Hello!

in ov.h inside interpreter, why is fast_elem_insert implemented as a
virtual method - by my knowledge no class inherits octave_value. This also
creates a virtual table for octave_value and makes octave_value twice the
size (instead of the size of one pointer, it is the size of two pointers).

Ps. this is my first time writing to a technical mailing list, so please
forgive any wrongdoing.
Interesting catch. All the inheritance happens within "octave_value"s representation `rep` attribute of type "octave_base_value". The method "fast_elem_insert" was added seven years ago [1] and since then not really changed. You suggest to just delete that keyword? How did you spot this one, you noticed performance issues or alike?

Kai.

[1]: https://hg.savannah.gnu.org/hgweb/octave/rev/654fbde5dceb
Reply | Threaded
Open this post in threaded view
|

Re: ov.h fast_elem_insert

gareins
Exactly.

Well, I have been hacking on Octave a bit. And yes this was an attempt to improve performance. Make check passes, but otherwise no visible performance improvement, that I can find.
Reply | Threaded
Open this post in threaded view
|

Re: ov.h fast_elem_insert

siko1056
gareins wrote
Exactly.

Well, I have been hacking on Octave a bit. And yes this was an attempt to improve performance. Make check passes, but otherwise no visible performance improvement, that I can find.
I removed the "virtual" keyword that to my knowledge makes no sense there and confirm that make check passes. That patch is quickly applied to the default branch. Thank you for your observation. Do you want to get credited for the changeset? (What name and e-mail address should appear at [1]?)

Kai.

[1]: https://hg.savannah.gnu.org/hgweb/octave/graph/