Bradley Smith wrote:
> 
> 
> Dave wrote:
>> %u wrote:
>>> Bill Baxter Wrote:
>>>> So the C++ code is ok.  But it's not clear why Material became a
>>>> class in the D version rather than a struct.
>>> Thx. I did not notice, that "Material" is a struct in the cpp-version.
>>>
>>> This shows however, that programmers still are not following engeering principles: no technical documentation of the port is given and no one complains.
>>>
>>> Instead several people are eager searching flaws in the reference implementation of D for which there is also no technical documentation :-(
>>
>> Let's assume that the OP was earnestly trying to make the C++ and D code comparable... If so, then this exercise did point out some areas where D needs attention. In the final analysis, it's "good faith" ports like these that are going to satisfy whether or not D "is as fast or faster" than C++, and in many cases, whether or not people will make the switch. If it requires a lot of code modifications over and above a simple port to make D comparable in performance, people will shy away from D.
> 
> Thanks for defending me, Dave. You are correct in assuming that I am trying to make the C++ and D code comparable. I'm not trying to sabotage the D effort. In fact, I would very much like to see the D code perform significantly better than C++. I'm just trying to learn how to write high-performance D code.
> 
> Thanks,
>   Bradley


I think this was a great little benchmark you posted.  I hope Walter takes some interest in this too, because he's consistently responded to performance questions with "I bet it'll be the same if you compile with DMC and DMD".  But now at last we have a real-world kind of benchmark with which to test that assertion.  The answer appears to be negative at the moment, but just as with bugs, you can't fix it if you can't reproduce the problem.  And you've given us a very nice repro case.

--bb

Bill Baxter wrote:
> %u wrote:
>> Bill Baxter Wrote:
>>> D still not as fast as the C++, but close.
>>
>> I refuse to analyze this any further.
>>
>> On comparing the implementations of Primary, I noticed, that the OP has introduced a constructor which executes "new Material". There is no "new" in the cpp-version of Primary but a "SetMaterial" function.
>>
>> On deleting the new expression in the D-version an exception was raised on executing the newly compiled binary.
>>
>> Astonishingly grepping over the .cpp and .h -files with agent ransack no calls of "SetMaterial" were delivered---but "GetMaterial" is called---which uses the unset "Material" pointer. :-(
>>
>> Conclusion: at least one of the following is true
>> 1) I have near to no ability to understand c++ 2) the c++-version is lucky to run at all
>>
>> In case of 2) the OP has silently changed the algorithm on porting to D.
> 
> It's case 1) I'm afraid.  :-)
> 
> Material is a by-value member of Primitive in the C++ version.  This means it acts more like a D struct than a D class.  GetMaterial calls return a pointer to the Material that's part of the class, and it will have been initialized implicitly by the Primitive constructor using whatever Material's default constructor does.
>
> So the C++ code is ok.  But it's not clear why Material became a class in the D version rather than a struct.

Because in the C++, GetMaterial returns a pointer. Since other objects can use the pointer to change the value of the Material contained within a Primitive, the same behavior was used in the D code by using a class. If a struct had been used, a copy of Material would be returned, and changing the Material would have no effect on the Primitive.

Also, because GetMaterial is called very often, I assume that making lots of copies of it would decrease performance. Presumably, that is why the C++ code returns a pointer.

Thanks,
  Bradley

%u wrote:
> Bill Baxter Wrote:
>> So the C++ code is ok.  But it's not clear why Material became a
>> class in the D version rather than a struct.
> Thx. I did not notice, that "Material" is a struct in the cpp-version.
> 
> This shows however, that programmers still are not following engeering principles: no technical documentation of the port is given and no one complains.
> 
> Instead several people are eager searching flaws in the reference implementation of D for which there is also no technical documentation :-(

What technical documentation would be proper? What would it contain?

Dave wrote:
> Bradley Smith wrote:
>>   Converted templates to regular functions. (Templates not being inlined)
> 
> Are you sure? I know templates can be/are inlined and I guess I haven't noticed anywhere they aren't were I'd expect a regularly defined function to be inlined.

No, I'm not sure. I'm assuming based on the performance increase when they are manually inlined. It could very well be that template functions are inlined as much as regular functions, since the regular functions weren't being inlined either.

Thanks,
  Bradley

I think this thread is worth posting as a (D-performance) tutorial or
something.
Alot of interesting performance issues have come up, of which most were
unknown to me :)

What do you think?

Bradley Smith wrote:
> 
> 
> Bill Baxter wrote:
>> %u wrote:
>>> Bill Baxter Wrote:

>> Material is a by-value member of Primitive in the C++ version.  This means it acts more like a D struct than a D class.  GetMaterial calls return a pointer to the Material that's part of the class, and it will have been initialized implicitly by the Primitive constructor using whatever Material's default constructor does.
>>
>> So the C++ code is ok.  But it's not clear why Material became a class in the D version rather than a struct.
> 
> Because in the C++, GetMaterial returns a pointer. Since other objects can use the pointer to change the value of the Material contained within a Primitive, the same behavior was used in the D code by using a class. If a struct had been used, a copy of Material would be returned, and changing the Material would have no effect on the Primitive.
> 
> Also, because GetMaterial is called very often, I assume that making lots of copies of it would decrease performance. Presumably, that is why the C++ code returns a pointer.

You can return pointers in D too.  But anyway, I don't think the change from by-value class in C++ to a by-reference class in D made any difference in the runtime.  I wasn't saying that it was wrong that you changed Material to a D class or anything.  It's a valid approach and certainly more D-ish than returning a pointer to a struct.

--bb

Bradley Smith Wrote:
> Thanks for defending me, Dave.
At least me was not attacking you. I was mostly attacking myself to fall victim to a known source of errors.

As Bill Baxter pointed out, I missed an optimization on version 2. The pass by reference optimization using the inout on the Intersect's Ray argument. I had applied inout only to the Raytrace's Ray argument.

The further optimization brings the following approx. timings:

         time     factor
   dmc    5 sec    1.0
   dmd    9 sec    1.8
   gdc    13 sec   2.6
   msvc   5 sec    1.0
   g++    doesn't compile

Version 3 is attached and has the following changes:
   Fixed compiling with gdc
   Use inout for Intersect's Ray argument

Thanks,
   Bradley

Bradley Smith wrote:
> Jacco Bikker wrote several raytracing articles on DevMaster.net. I took his third article and ported it to D. I was surprised to find that the D code is approx. 4 times slower than C++.
> 
> The raytracer_d renders in approx. 21 sec and the raytracer_cpp renders in approx. 5 sec. I am using the DMD and DMC compilers on Windows.
> 
> How can the D code be made to run faster?
> 
> Thanks,
>   Bradley
> 

Bradley Smith Wrote:
> What technical documentation would be proper? What would it contain?

As always such depends on the requirements of the presumed readers.

If you are able to change your position from the view of the porter to the view of a verifier or freshly introduced maintainer of the port, then you will have an impression of what you would want to look at first.

It is a pity as it stands, that the question for the content of the technical documentation raises at all.

For example the answer you gave to Bill Baxter:

| Because in the C++, GetMaterial returns a pointer. Since other
| objects can use the pointer to change the value of the Material
| contained within a Primitive, the same behavior was used in the D
| code by using a class. If a struct had been used, a copy of Material
| would be returned, and changing the Material would have no effect on
| the Primitive.

| Also, because GetMaterial is called very often, I assume that making | lots of copies of it would decrease performance. Presumably, that
| is why the C++ code returns a pointer.

would belong into such documentation as well as any other decision that was made during the port.

For example I found a ".dup" in the D-version where there was no copying in the cpp-version. The question raises immediately whether this is done with intent or by accident. Without redundancy provided by technical documentation a careful analysis for the necessity of these four characters has to be undertaken.

Bradley Smith wrote:
> As Bill Baxter pointed out, I missed an optimization on version 2. The pass by reference optimization using the inout on the Intersect's Ray argument. I had applied inout only to the Raytrace's Ray argument.
> 
> The further optimization brings the following approx. timings:
> 
>         time     factor
>   dmc    5 sec    1.0
>   dmd    9 sec    1.8
>   gdc    13 sec   2.6
    gdc    10 sec   2.0      <-- correction
>   msvc   5 sec    1.0
>   g++    doesn't compile
> 

Here is a correction to the gdc results. The wrong optimization flag was used. The build_d_gdc.bat should have "-O3" rather than "-O".