Re: [Patch, fortran] PR69834 - Collision in derived type hashes

[Paul Richard Thomas <paul dot richard dot thomas at gmail dot com>]

Dear All,

After submitting the patch, I did something that I should have done a
long time ago: some timing tests :-)

I used actual_array_offset_1.f90, which is based on Arjen Markus's
implementation of quicksort using unlimited polymorphic entities as
carriers of the objects to be sorted.

With 10^5 elements in the array and -O3, without the patch, the
execution time is 49ms. With the patch it climbs to 315ms

Dominique repeated the test with 10^7 elements and got 4.4s before the
patch and 46.5 after.

In light of this, I withdraw the submission and will concentrate on
making the pointer version work in all circumstances with submodules.

Best regards

Paul

On 27 September 2016 at 10:27, Paul Richard Thomas
<paul.richard.thomas@gmail.com> wrote:
> Dear All,
>
> The first attempts at fixing this bug were posted to the PR in
> February of this year. Since then, real life has intervened and I have
> not been able to get back to it until now.
>
> The first patch used the address of the vtable to perform the
> switching in SELECT_TYPE. Unfortunately, it failed in submodule_6.f90
> and I have not been able to find a way to fix this without breaking
> the ABI and having to bump up the module version number.
>
> The second patch uses a string for the switching, which comprises a
> concatenation of the type name and the module or procedure name.
> Clearly, there is a performance penalty associated with this. My
> recent efforts have been focussed on making this version detect
> incoming selectors and associates that are use associated with
> libraries that were compiled before this patch was applied and the
> result is this submission. By the way, I was unable to find a way of
> testing this feature as part of the testsuite but have done so 'by
> hand'.
>
> If the performance penalty is considered to be a show stopper, I could
> develop further the version based on the vtable addresses but will
> have to postpone any further work on this for a few weeks.
>
> Otherwise, this patch does bootstrap and regtest on FC21/x86_64 - OK for trunk?
>
> Cheers
>
> Paul
>
> 2016-09-27  Paul Thomas  <pault@gcc.gnu.org>
>
>     PR fortran/69834
>     * class.c (get_unique_type_string): Add an extra argument
>     'icase' that defaults to false but, when true, switches the
>     order of type name and module or procedure name.
>     (get_unique_hashed_string): New argument 'icase' switches
>     bewteen the old form and a new one in which the string length
>     is limited to GFC_MAX_SYMBOL_LEN and, in case of this limit
>     being exceeded, the hash string is followed by as much of the
>     composite name as possible.
>     (gfc_case_name): New function.
>     (gfc_find_derived_vtab): Add '_name' field to vtable. This is
>     initialized by 'get_unique_type_string' with 'icase' true.
>     (find_intrinsic_vtab): Ditto with initialization performed by a
>     call to 'gfc_case_name'.
>     * gfortran.h : Add macro 'gfc_add_name_component' and prototype
>     for 'gfc_case_name'.
>     * resolve.c (vtable_old_style): New function to determine if a
>     use associated vtable is missing the '_name' field.
>     (resolve_select_type): Call 'vtable_old_style' to determine if
>     any of the derived types or vtables come from a library that
>     was compiled before this patch. If this is the case, the old
>     form of SELECT TYPE is activated, in which the cases are set by
>     the hash value. Otherwise, the 'unique_type_string' is used.
>
> 2016-09-27  Paul Thomas  <pault@gcc.gnu.org>
>
>     PR fortran/69834
>     * gfortran.dg/finalize_21.f90: Remove semi colon from the tree
>     scan.
>     * gfortran.dg/select_type_36.f03: New test.
>     * gfortran.dg/select_type_37.f03: New test.



-- 
The difference between genius and stupidity is; genius has its limits.

Albert Einstein