octave-maintainers
[Top][All Lists]
Advanced

[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


reply via email to

[Prev in Thread] Current Thread [Next in Thread]