On 06/21/2010 11:22 AM, strk wrote:
> Not really.
> You're right about additional cost, but I dubt it'll really
> be noticeable. Have some profiles ?
Not directly on geos but testing the same scenario against a trivial
example (attached) gives these results on gcc 4.4.3 :
$ g++ -O0 -o bench bench.cc && ./bench
elapsed: 00:00:24.000799
elapsed: 00:00:27.809378
diff: 00:00:03.808579
added cost: 15.8686%
$ g++ -O2 -o bench bench.cc && ./bench
elapsed: 00:00:13.456086
elapsed: 00:00:14.927550
diff: 00:00:01.471464
added cost: 10.9353%
$ g++ -O3 -o bench bench.cc && ./bench
elapsed: 00:00:01.757662
elapsed: 00:00:02.050734
diff: 00:00:00.293072
added cost: 16.674%
First 'elapsed' is an unsafe access, second is the same access protected
by an if().
It is clearly noticeable in any optimization level.
> We could move the checks in higher level APIs and
> leave internals alone. Note that the C-API says it'd return
> 0 on exception. Isn't asking for an non-existant child
> an exceptional request ? Should the C-API functions check ?
It is indeed an exceptional request and this is why I find that paying
the cost of this additional safety for exceptional cases is too much.
These methods are typically used in loop-intensive contexts.
Furthermore, they add no safety when used in loops such as :
for(std::size_t i = 0; i < geom->getNumGeometries(); ++i)
// ...geom->getGeometryN(i)...
Having a safe and slow API is fine, std::vector has one, but not having
an unsafe bust fast API is bad for those who care about performance.
These functions are quite fundamental and broadly used in geos, this is
going to slow down everything a bit.
As I don't use the C-API I don't know if it should do the test or not.
I'd say that there should also be two functions, unsafe & fast and safe
& slow. By the way, a lot of C code has the 'unsafe & fast' behaviour.
Typically the libc : str{len,cpy,...} functions will segfault if they
are given invalid arguments (NULL pointers, ...). So a C developer
should be used to functions that fail when given invalid arguments.
--
Maxime
#include <iostream>
#include <vector>
#include <boost/date_time/posix_time/posix_time.hpp>
boost::posix_time::time_duration elapsed(const boost::posix_time::ptime& start)
{
boost::posix_time::time_duration duration = boost::posix_time::microsec_clock::local_time() - start;
std::cout << "elapsed: " << duration << std::endl;
return duration;
}
void set(std::vector<int>& vect, std::size_t ind, int val)
{
vect[ind] += val;
}
void safe_set(std::vector<int>& vect, std::size_t ind, int val)
{
if (ind > vect.size())
return;
vect[ind] += val;
}
void safe_set2(std::vector<int>& vect, std::size_t ind, int val)
{
vect.at(ind) = val;
}
int main(int argc, char* argv[])
{
std::size_t nb_loops = 1000000000;
std::vector<int> v(10, 42);
// Cache
for (std::size_t i = 0; i < nb_loops; ++i)
v[i % v.size()] = 42;
boost::posix_time::time_duration d1, d2;
// Without if()
{
const boost::posix_time::ptime& start = boost::posix_time::microsec_clock::local_time();
for (std::size_t i = 0; i < nb_loops; ++i)
set(v, (i + argc) % v.size(), argc);
d1 = elapsed(start);
}
// With if()
{
const boost::posix_time::ptime& start = boost::posix_time::microsec_clock::local_time();
for (std::size_t i = 0; i < nb_loops; ++i)
safe_set(v, (i + argc) % v.size(), argc);
d2 = elapsed(start);
}
std::cout << "diff: " << d2 - d1 << std::endl
<< "added cost: " << (double) d2.total_microseconds() / (double) d1.total_microseconds() * 100. - 100 << "%" << std::endl;
}
_______________________________________________
geos-devel mailing list
geos-devel@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/geos-devel
