[
https://issues.apache.org/jira/browse/GEODE-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17132617#comment-17132617
]
Blake Bender edited comment on GEODE-8212 at 6/10/20, 6:56 PM:
---------------------------------------------------------------
Okay, here's the latest:
* First, the call to PdxInstanceFactory::writeString was being called in the
order (value, key) rather than (key, value). This meant that _literally_ every
iteration was producing a new Pdx type. Swapping the order from:
{code:java}
auto repetitions = 5000;
for (auto i = 0; i < repetitions; ++i) {
auto instance_factory =
cache.createPdxInstanceFactory(gemfireJsonClassName);
instance_factory.writeString(std::to_string(i), "bar");
}{code}
to:
{code:java}
auto repetitions = 5000;
for (auto i = 0; i < repetitions; ++i) {
auto instance_factory =
cache.createPdxInstanceFactory(gemfireJsonClassName);
instance_factory.writeString("bar", std::to_string(i));
} {code}
Took the timing way down, as shown below:
before:
{code:java}
○ → ctest -R testCreateInstanceFactory -VV 2>&1 | grep Elapsed
30: Elapsed Time (createPdxInstanceFactory): 17 milliseconds
30: Elapsed Time (instance_factory.writeString): 5 milliseconds
30: Elapsed Time (instance_factory.create): 12817 milliseconds
30: Elapsed Time (whole test): 12854 milliseconds{code}
after:
{code:java}
○ → ctest -R testCreateInstanceFactory -VV 2>&1 | grep Elapsed
30: Elapsed Time (createPdxInstanceFactory): 9 milliseconds
30: Elapsed Time (instance_factory.writeString): 2 milliseconds
30: Elapsed Time (instance_factory.create): 2299 milliseconds
30: Elapsed Time (whole test): 2328 milliseconds {code}
Then, I realized that I had turned on debug-level logging as part of my
investigation, so my results were probably still too high. Turning logging
off, I got the following:
{code:java}
○ → ctest -R testCreateInstanceFactory -VV 2>&1 | grep Elapsed
30: Elapsed Time (createPdxInstanceFactory): 10 milliseconds
30: Elapsed Time (instance_factory.writeString): 1 milliseconds
30: Elapsed Time (instance_factory.create): 442 milliseconds
30: Elapsed Time (whole test): 471 milliseconds {code}
I checked to see if perhaps something in the code was confusing compiler
optimization, and made a change I think we'll keep anyway because it makes the
operation more explicit.
before:
{code:java}
auto typeIdLessThan = this->m_geodeTypeId < other.m_geodeTypeId;
auto typeIdsBothZero = (m_geodeTypeId == 0) && (other.m_geodeTypeId == 0);
auto classnameLessThan = this->m_className < other.m_className;
return (typeIdLessThan || (typeIdsBothZero && classnameLessThan)); {code}
after:
{code:java}
if (m_geodeTypeId < other.m_geodeTypeId) {
return true;
}
if ((m_geodeTypeId == 0) && (other.m_geodeTypeId == 0)) {
return this->m_className < other.m_className;
}
return false; {code}
Unsurprisingly, this made no discernible difference in the timing.
I also tried moving `operator<` into the header and marking it inline, just to
be absolutely sure. Again, no discernible improvement.
At this point, I believe we can lower the priority of this bug. The
performance difference is still a thing, but again the prior implementation was
_wrong_, and fast but wrong is still wrong. I believe the current
implementation is as slow as it has to be, and no slower.
was (Author: bbender):
Okay, here's the latest:
* First, the call to
{code:java}
auto repetitions = 5000;
for (auto i = 0; i < repetitions; ++i) {
auto instance_factory =
cache.createPdxInstanceFactory(gemfireJsonClassName);
instance_factory.writeString("bar", std::to_string(i));{code}
> Change in PdxType class "<" operator impacts performance
> --------------------------------------------------------
>
> Key: GEODE-8212
> URL: https://issues.apache.org/jira/browse/GEODE-8212
> Project: Geode
> Issue Type: Improvement
> Components: native client
> Reporter: Alberto Bustamante Reyes
> Assignee: Blake Bender
> Priority: Major
> Attachments: performance_after_GEODE-7694.patch
>
>
> While investigating a performance problem of the C++ native client, it has
> been observed that change in the PdxType class "<" operator introduced by
> GEODE-7694 impacts performance of PdxHelper::serializePdx().
> Im attaching a patch with some code and a test case I have used to benchmark
> that method. With the current implementation in develop branch, creating just
> 10 instances is enough to see the difference. The calls to serializePdx take
> these times:
> {noformat}
> Elapsed Time (serializePdx): 32.1322 milliseconds
> Elapsed Time (serializePdx): 3.06044 milliseconds
> Elapsed Time (serializePdx): 1.58791 milliseconds
> Elapsed Time (serializePdx): 1.62466 milliseconds
> Elapsed Time (serializePdx): 1.53676 milliseconds
> Elapsed Time (serializePdx): 1.59278 milliseconds
> Elapsed Time (serializePdx): 1.82878 milliseconds
> Elapsed Time (serializePdx): 1.36811 milliseconds
> Elapsed Time (serializePdx): 1.6956 milliseconds
> Elapsed Time (serializePdx): 1.46873 milliseconds
> Elapsed Time (serializePdx): 1.53206 milliseconds
> {noformat}
> While these are the times using the old "<" operator:
> {noformat}
> Elapsed Time (serializePdx): 56.1524 milliseconds
> Elapsed Time (serializePdx): 0.012327 milliseconds
> Elapsed Time (serializePdx): 0.006325 milliseconds
> Elapsed Time (serializePdx): 0.005419 milliseconds
> Elapsed Time (serializePdx): 0.005266 milliseconds
> Elapsed Time (serializePdx): 0.005662 milliseconds
> Elapsed Time (serializePdx): 0.005407 milliseconds
> Elapsed Time (serializePdx): 0.005286 milliseconds
> Elapsed Time (serializePdx): 0.005467 milliseconds
> Elapsed Time (serializePdx): 0.005276 milliseconds
> Elapsed Time (serializePdx): 0.005298 milliseconds
> {noformat}
> And after creating 50.000 instances, these are the times for the last 10
> calls with the current "<" operator and with the old one respectively::
> {noformat}
> Elapsed Time (serializePdx): 8.11767 milliseconds
> Elapsed Time (serializePdx): 8.15727 milliseconds
> Elapsed Time (serializePdx): 8.09303 milliseconds
> Elapsed Time (serializePdx): 7.89909 milliseconds
> Elapsed Time (serializePdx): 8.37956 milliseconds
> Elapsed Time (serializePdx): 8.21303 milliseconds
> Elapsed Time (serializePdx): 8.14189 milliseconds
> Elapsed Time (serializePdx): 8.21266 milliseconds
> Elapsed Time (serializePdx): 8.21683 milliseconds
> Elapsed Time (serializePdx): 7.78765 milliseconds
> {noformat}
> {noformat}
> Elapsed Time (serializePdx): 0.003014 milliseconds
> Elapsed Time (serializePdx): 0.003 milliseconds
> Elapsed Time (serializePdx): 0.003084 milliseconds
> Elapsed Time (serializePdx): 0.003112 milliseconds
> Elapsed Time (serializePdx): 0.00305 milliseconds
> Elapsed Time (serializePdx): 0.003018 milliseconds
> Elapsed Time (serializePdx): 0.003029 milliseconds
> Elapsed Time (serializePdx): 0.003061 milliseconds
> Elapsed Time (serializePdx): 0.003043 milliseconds
> Elapsed Time (serializePdx): 0.003086 milliseconds
> {noformat}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
