[jira] [Commented] (IGNITE-2744) Optimize "unwindEvict" call in GridCacheIoManager.processMessage().
[ https://issues.apache.org/jira/browse/IGNITE-2744?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15296370#comment-15296370 ] ASF GitHub Bot commented on IGNITE-2744: Github user asfgit closed the pull request at: https://github.com/apache/ignite/pull/647 > Optimize "unwindEvict" call in GridCacheIoManager.processMessage(). > --- > > Key: IGNITE-2744 > URL: https://issues.apache.org/jira/browse/IGNITE-2744 > Project: Ignite > Issue Type: Task > Components: cache >Affects Versions: 1.5.0.final >Reporter: Vladimir Ozerov >Assignee: Semen Boikov >Priority: Critical > Labels: performance > Fix For: 1.7 > > > We call this method on every (!!!) received cache message. This call is > pretty heavy as it iterates over all caches. > We need to optimize it. E.g., check evicts only for the cache to which > received message belongs. And iterate over the whole set only if we know for > sure that several caches are affected (e.g. due to cross-cache TX). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (IGNITE-2744) Optimize "unwindEvict" call in GridCacheIoManager.processMessage().
[ https://issues.apache.org/jira/browse/IGNITE-2744?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15286159#comment-15286159 ] Semen Boikov commented on IGNITE-2744: -- Reviewed, last comments: - processing of GridCacheTxRecoveryRequest can cause tx commit so it makes sense to support txState fot this message - let's add some marker interfaces for messages with txState and use one 'instanceof' in GridCacheIoManager.processMessage > Optimize "unwindEvict" call in GridCacheIoManager.processMessage(). > --- > > Key: IGNITE-2744 > URL: https://issues.apache.org/jira/browse/IGNITE-2744 > Project: Ignite > Issue Type: Task > Components: cache >Affects Versions: 1.5.0.final >Reporter: Vladimir Ozerov >Assignee: Semen Boikov >Priority: Critical > Labels: performance > Fix For: 1.6 > > > We call this method on every (!!!) received cache message. This call is > pretty heavy as it iterates over all caches. > We need to optimize it. E.g., check evicts only for the cache to which > received message belongs. And iterate over the whole set only if we know for > sure that several caches are affected (e.g. due to cross-cache TX). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (IGNITE-2744) Optimize "unwindEvict" call in GridCacheIoManager.processMessage().
[
https://issues.apache.org/jira/browse/IGNITE-2744?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15282729#comment-15282729
]
Ilya Lantukh commented on IGNITE-2744:
--
I tried to inject tx state into message in the most 'top level' handler.
Unfortunately, for some message types TX lookup isn't as simple as for
NearTxPrepareResponse.
I think the simplest way how we can test it is to add {code} assert
msg.txState() != null; {code} to the end of every top level handler.
> Optimize "unwindEvict" call in GridCacheIoManager.processMessage().
> ---
>
> Key: IGNITE-2744
> URL: https://issues.apache.org/jira/browse/IGNITE-2744
> Project: Ignite
> Issue Type: Task
> Components: cache
>Affects Versions: 1.5.0.final
>Reporter: Vladimir Ozerov
>Assignee: Semen Boikov
>Priority: Critical
> Labels: performance
> Fix For: 1.6
>
>
> We call this method on every (!!!) received cache message. This call is
> pretty heavy as it iterates over all caches.
> We need to optimize it. E.g., check evicts only for the cache to which
> received message belongs. And iterate over the whole set only if we know for
> sure that several caches are affected (e.g. due to cross-cache TX).
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (IGNITE-2744) Optimize "unwindEvict" call in GridCacheIoManager.processMessage().
[ https://issues.apache.org/jira/browse/IGNITE-2744?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15282713#comment-15282713 ] Semen Boikov commented on IGNITE-2744: -- Here are my comments: - IgniteTxHandler's methods 'processNearTxPrepareResponse', 'processDhtTxPrepareResponse' it should be possible to get tx from cache future you already have and avoid 'tm().tx' call - error in IgniteTxImplicitSingleStateImpl.unwindEvicts: 'if (entry == null && entry.isEmpty())' need change to 'if (entry == null || entry.isEmpty())' It is hard to tell is tx state is really initialized for all messages, even it is works now it can be easily broken after code changes. Can we add test which check that tx state is really set for all messages? Also it would be more easy to understand this change if tx state was always set in some 'top level' message process message (like it is done for example in 'processNearTxPrepareResponse'). Is it possible to change code this way? > Optimize "unwindEvict" call in GridCacheIoManager.processMessage(). > --- > > Key: IGNITE-2744 > URL: https://issues.apache.org/jira/browse/IGNITE-2744 > Project: Ignite > Issue Type: Task > Components: cache >Affects Versions: 1.5.0.final >Reporter: Vladimir Ozerov >Assignee: Semen Boikov >Priority: Critical > Labels: performance > Fix For: 1.6 > > > We call this method on every (!!!) received cache message. This call is > pretty heavy as it iterates over all caches. > We need to optimize it. E.g., check evicts only for the cache to which > received message belongs. And iterate over the whole set only if we know for > sure that several caches are affected (e.g. due to cross-cache TX). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (IGNITE-2744) Optimize "unwindEvict" call in GridCacheIoManager.processMessage().
[ https://issues.apache.org/jira/browse/IGNITE-2744?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15281482#comment-15281482 ] Ilya Lantukh commented on IGNITE-2744: -- 2. We couldn't find any place in code where new entry is inserted into that map and assertion for readMap.isEmpty() never failed. I think it proves that map is never updated. 3. Sure, I will change it. > Optimize "unwindEvict" call in GridCacheIoManager.processMessage(). > --- > > Key: IGNITE-2744 > URL: https://issues.apache.org/jira/browse/IGNITE-2744 > Project: Ignite > Issue Type: Task > Components: cache >Affects Versions: 1.5.0.final >Reporter: Vladimir Ozerov >Assignee: Ilya Lantukh >Priority: Critical > Labels: performance > Fix For: 1.6 > > > We call this method on every (!!!) received cache message. This call is > pretty heavy as it iterates over all caches. > We need to optimize it. E.g., check evicts only for the cache to which > received message belongs. And iterate over the whole set only if we know for > sure that several caches are affected (e.g. due to cross-cache TX). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (IGNITE-2744) Optimize "unwindEvict" call in GridCacheIoManager.processMessage().
[
https://issues.apache.org/jira/browse/IGNITE-2744?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15281470#comment-15281470
]
Vladimir Ozerov commented on IGNITE-2744:
-
Looked at implementation again. I can neither accept, not reject it. My
concerns:
1) I can say nothing about changes to {{IgniteTxHandler}} as I am not familiar
with this piece of code.
2) {{IgniteTxRemoteStateImpl}} - looks readMap is always empty and is never
filled with any data. Is it true?
3) {{IgniteTxRemoteStateImpl}} - having separate concurrent HM seems too heavy
for me. Normally user will have 1 cache, rarely 2, very rare - more than 2.
Wrapped CHM looks like an overkill for me. We already produce too much garbage
and I do not want to add more. Can we do something like this (pseudocode)?
{code}
public void unwindEvicts(GridCacheSharedContext cctx) {
assert readMap == null || readMap.isEmpty();
int singleCacheId = -1;
Set cacheIds = null;
for (IgniteTxKey writeKey : writeMap.keySet()) {
int cacheId = writeKey.cacheId();
// Have we already notified this cache?
if (cacheId == singleCacheId || cacheIds != null &&
!cacheIds.add(writeCacheId))
continue;
if (singleCacheId == -1)
singleCacheId = cacheId;
if (cacheIds == null) {
cacheIds = new HashSet<>(2);
cacheIds.add(cacheId);
}
GridCacheContext ctx = cctx.cacheContext(cacheId);
if (ctx != null)
CU.unwindEvicts(ctx);
}
}
{code}
> Optimize "unwindEvict" call in GridCacheIoManager.processMessage().
> ---
>
> Key: IGNITE-2744
> URL: https://issues.apache.org/jira/browse/IGNITE-2744
> Project: Ignite
> Issue Type: Task
> Components: cache
>Affects Versions: 1.5.0.final
>Reporter: Vladimir Ozerov
>Assignee: Vladimir Ozerov
>Priority: Critical
> Labels: performance
> Fix For: 1.6
>
>
> We call this method on every (!!!) received cache message. This call is
> pretty heavy as it iterates over all caches.
> We need to optimize it. E.g., check evicts only for the cache to which
> received message belongs. And iterate over the whole set only if we know for
> sure that several caches are affected (e.g. due to cross-cache TX).
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (IGNITE-2744) Optimize "unwindEvict" call in GridCacheIoManager.processMessage().
[ https://issues.apache.org/jira/browse/IGNITE-2744?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15280975#comment-15280975 ] Ilya Lantukh commented on IGNITE-2744: -- Fixed. > Optimize "unwindEvict" call in GridCacheIoManager.processMessage(). > --- > > Key: IGNITE-2744 > URL: https://issues.apache.org/jira/browse/IGNITE-2744 > Project: Ignite > Issue Type: Task > Components: cache >Affects Versions: 1.5.0.final >Reporter: Vladimir Ozerov >Assignee: Ilya Lantukh >Priority: Critical > Labels: performance > Fix For: 1.6 > > > We call this method on every (!!!) received cache message. This call is > pretty heavy as it iterates over all caches. > We need to optimize it. E.g., check evicts only for the cache to which > received message belongs. And iterate over the whole set only if we know for > sure that several caches are affected (e.g. due to cross-cache TX). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (IGNITE-2744) Optimize "unwindEvict" call in GridCacheIoManager.processMessage().
[ https://issues.apache.org/jira/browse/IGNITE-2744?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15248030#comment-15248030 ] Ilya Lantukh commented on IGNITE-2744: -- Turns out other TX messages should call unwindEvicts() for involved caches too. > Optimize "unwindEvict" call in GridCacheIoManager.processMessage(). > --- > > Key: IGNITE-2744 > URL: https://issues.apache.org/jira/browse/IGNITE-2744 > Project: Ignite > Issue Type: Task > Components: cache >Affects Versions: 1.5.0.final >Reporter: Vladimir Ozerov >Assignee: Ilya Lantukh >Priority: Critical > Labels: performance > Fix For: 1.6 > > > We call this method on every (!!!) received cache message. This call is > pretty heavy as it iterates over all caches. > We need to optimize it. E.g., check evicts only for the cache to which > received message belongs. And iterate over the whole set only if we know for > sure that several caches are affected (e.g. due to cross-cache TX). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (IGNITE-2744) Optimize "unwindEvict" call in GridCacheIoManager.processMessage().
[ https://issues.apache.org/jira/browse/IGNITE-2744?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15245652#comment-15245652 ] Ilya Lantukh commented on IGNITE-2744: -- Failed some tests related to off-heap storage. > Optimize "unwindEvict" call in GridCacheIoManager.processMessage(). > --- > > Key: IGNITE-2744 > URL: https://issues.apache.org/jira/browse/IGNITE-2744 > Project: Ignite > Issue Type: Task > Components: cache >Affects Versions: 1.5.0.final >Reporter: Vladimir Ozerov >Assignee: Ilya Lantukh >Priority: Critical > Labels: performance > Fix For: 1.6 > > > We call this method on every (!!!) received cache message. This call is > pretty heavy as it iterates over all caches. > We need to optimize it. E.g., check evicts only for the cache to which > received message belongs. And iterate over the whole set only if we know for > sure that several caches are affected (e.g. due to cross-cache TX). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (IGNITE-2744) Optimize "unwindEvict" call in GridCacheIoManager.processMessage().
[ https://issues.apache.org/jira/browse/IGNITE-2744?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15245597#comment-15245597 ] Ilya Lantukh commented on IGNITE-2744: -- Changed processMessage() implementation to call unwindEvicts() only for a single cacheId to which processed message belongs. The only special case is handling of GridDistributedTxFinishRequests: now all involved cacheIds will be stored during transaction processing, and then unwindEvicts() will be called only for those stored IDs. Tested performance of these optimizations in 4 server 16 client environment: throughput of single put/get operations has improved by 4% for atomic caches and by 1% for transactional. > Optimize "unwindEvict" call in GridCacheIoManager.processMessage(). > --- > > Key: IGNITE-2744 > URL: https://issues.apache.org/jira/browse/IGNITE-2744 > Project: Ignite > Issue Type: Task > Components: cache >Affects Versions: 1.5.0.final >Reporter: Vladimir Ozerov >Assignee: Ilya Lantukh >Priority: Critical > Labels: performance > Fix For: 1.6 > > > We call this method on every (!!!) received cache message. This call is > pretty heavy as it iterates over all caches. > We need to optimize it. E.g., check evicts only for the cache to which > received message belongs. And iterate over the whole set only if we know for > sure that several caches are affected (e.g. due to cross-cache TX). -- This message was sent by Atlassian JIRA (v6.3.4#6332)
