[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: template instantations
From: |
John W. Eaton |
Subject: |
Re: template instantations |
Date: |
Thu, 13 Nov 2008 12:02:28 -0500 |
On 13-Nov-2008, Jaroslav Hajek wrote:
| An interesting doc about instantiating in g++ is here:
| http://gcc.gnu.org/onlinedocs/gcc/Template-Instantiation.html
|
| It appears to say that the templates are reinstantiated and recompiled
| for each compilation unit including the file (unless you use the g++
| "extern template" extension). Maybe that contributes to the slow
| compiling of liboctave.
|
| The present approach compiles the non-inline member functions only
| once (in Array-*.cc).
OK, so we should probably keep using the style we have, where larger
methods are in separate files. But simple methods of just a few
instructions should probably remain in the header files so that they
can be inlined where possible.
| Maybe the problem is that Array<T> should not really be used for
| "lightweight" arrays, where you don't need the "fat" methods (index,
| assign, resize, sort).
Looking for places where we currently use Array<T> directly and
replacing them with std::vector or some other appropriate object would
be fine with me.
| Another point is that, IMHO, "sort" should not be part of Array<T> but
| rather MArray<T>. Array<T> should only require T to be copyable and
| default constructible. MArray<T> requires T to be comparable, so it's
| a good place for sort. That way, we could remove the
| NO_INSTANTIATE_ARRAY_SORT stuff.
I think a change like this would also be good.
| > If we do keep both, maybe we should follow the lead of the GNU
| > libstdc++ files and rename the .cc file .tcc, since it is not a normal
| > source file, but a set of templates. Also, the libstdc++ sources use
| > (for example)
| >
| > #ifndef _GLIBCXX_EXPORT_TEMPLATE
| > # include <bits/basic_string.tcc>
| > #endif
| >
| > so I think the basic_string.tcc file is normally included in the
| > <string> header file.
| >
|
| A good suggestion.
OK, I think I'll make this change for all template classes that
currently have .h and .cc files, but I will continue to leave them
separate. Then the .tcc file should only be included (along with the
corresponding .h file) when we need the full definition of all methods
in a template class.
| > To see what changes like this might do, I took a shot at eliminating
| > the files {u,}int{8,16,32,64}NDArray.cc and merging the intNDArray.cc
| > file into the existing intNDArray.h header file. I'm attaching the
| > diffs below. With these changes, all information about the
| > intNDArray<T> type is in the single header file. It is now 859 lines
| > long.
| >
| > So, does anyone object to these changes?
| >
|
| Since intNDArray is only a thin wrapper over Array<octave_int<T> >, I
| think it's OK.
OK, I'll propose another patch that leaves the interface and
implementation mostly separate, and also moves the declarations for
these functions
NDS_CMP_OP_DECLS (int8NDArray, octave_int8, OCTAVE_API)
NDS_BOOL_OP_DECLS (int8NDArray, octave_int8, OCTAVE_API)
SND_CMP_OP_DECLS (octave_int8, int8NDArray, OCTAVE_API)
SND_BOOL_OP_DECLS (octave_int8, int8NDArray, OCTAVE_API)
NDND_CMP_OP_DECLS (int8NDArray, int8NDArray, OCTAVE_API)
NDND_BOOL_OP_DECLS (int8NDArray, int8NDArray, OCTAVE_API)
MARRAY_FORWARD_DEFS (MArrayN, int8NDArray, octave_int8)
MINMAX_DECLS (int8)
to the intNDArray.h file and makes them templates. The definitions
for these functions will go in the intNDArray.tcc file. Then the
{u,}int{8,16,32,64}NDArray.h files can simply provide the typedef, and
the {u,}int{8,16,32,64}NDArray.cc files can perform the explicit
instantiations for the classes and associated functions.
Does that sound OK?
| Personally, I'd like to see more code formed the way intNDArray is.
| For instance, having a single "Matrix" template class (say, MatrixT),
| and only typedefing MatrixT<double> to Matrix, MatrixT<float> to
| FloatMatrix etc. would simplify writing generic template functions
| supposed to work with both (or all four) kinds of matrices.
I think a change like this would be good, though maybe I would prefer
the name "base_matrix". We will still need to provide some
specializations for functions that call LAPACK or BLAS library
functions.
jwe