[jira] [Comment Edited] (IGNITE-4293) Deserialized value is cached if queries are enabled

["Alexandr Kuramshin (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/IGNITE-4293?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15751001#comment-15751001 ] 

Alexandr Kuramshin edited comment on IGNITE-4293 at 12/15/16 10:35 AM:
-----------------------------------------------------------------------

Let's look at the basis with two types: CacheObject and CacheObjectContext.

Because the implementations could be very different, let's think abstract.

Calling CacheObject.value(CacheObjectContext ctx, boolean cpy) we'll get a user object independently of underlying storage and implementation.

Supposed that the CacheObject MAY have a cached instance of the user object on-heap, and the <cpy> flag is like a hint controlling behaivor of such implementations. If it's true, then the copy of the user object MAY be returned, if underlying CacheObject have the such internal on-heap instance, but if it's false is also possible to retreive a new instance every time because of absence of internal field.

Next there is CacheObjectContext controlling the behavior of the caching on-heap instance and user objects will be returned. Precisely two methods: copyOnGet() and storeValue().

Supposed that copyOnGet() value controls behaviour of CacheObjects with respect to <cpy> flag. If copyOnGet() is false, then the cached user object should be returned, if the internal on-heap instance is present. And if storeValue() is true, then the CacheObject should store the user object upon deserialization, if the internal field is present.

It's seems that copyOnGet() and storeValue() are threated independently, but it's something wrong. If we'll call every time CacheObject.value(ctx, true) and both ctx.copyOnGet() and ctx.storeValue() are true, then the stored value will never be used. But relating to common usage of CacheObject.value method it's seems that usual value of <cpy> is false, so the problem is virtual.

I'm wondering that nothing of above was written in Javadoc.

Going further, I'm answering myself when the copyOnGet() and storeValue() should be true or false?

Definitly they should depend upon user preference CacheConfiguration.isCopyOnRead(): copyOnGet() == isCopyOnRead(); storeValue() != isCopyOnRead().

Also we should take in account how indexing are working. Whenever indexing is enabled on the cache, some number of GridH2ValueCacheObject are present (up to number of cache entries). There is the method GridH2ValueCacheObject.compareSecure(Value v, CompareMode mode) which compares the two CacheObject or its serialized user objects, depending of the CacheObject.isPlatformType() value. So, because compareSecure() method is time critical, we should set storeValue() to true, when indexing is enabled AND CacheObject.isPlatformType() is true for all entries in the cache, to avoid high CPU overhead upon deserializing user object on each compareSecure() invocation.

Actual CacheObject type stored in the cache depends on the IgniteConfiguration.getMarshaller() value configured. The GridQueryProcessor.isEnabled(CacheConfiguration ccfg) doesn't take into account what marshaller type is used, but only whether indexing is enabled. It should be fixed.

There is another condition IgniteConfiguration.isPeerClassLoadingEnabled() is used to determine CacheObjectContext.storeValue(). It is still being discovered.


was (Author: ein):
Let's look at the basis with two types: CacheObject and CacheObjectContext.

Because the implementations could be very different, let's think abstract.

Calling CacheObject.value(CacheObjectContext ctx, boolean cpy) we'll get a user object independently of underlying storage and implementation.

Supposed that the CacheObject MAY have a cached instance of the user object on-heap, and the {cpy} flag is like a hint controlling behaivor of such implementations. If it's true, then the copy of the user object MAY be returned, if underlying CacheObject have the such internal on-heap instance, but if it's false is also possible to retreive a new instance every time because of absence of internal field.

Next there is CacheObjectContext controlling the behavior of the caching on-heap instance and user objects will be returned. Precisely two methods: copyOnGet() and storeValue().

Supposed that copyOnGet() value controls behaviour of CacheObjects with respect to {cpy} flag. If copyOnGet() is false, then the cached user object should be returned, if the internal on-heap instance is present. And if storeValue() is true, then the CacheObject should store the user object upon deserialization, if the internal field is present.

It's seems that copyOnGet() and storeValue() are threated independently, but it's something wrong. If we'll call every time CacheObject.value(ctx, true) and both ctx.copyOnGet() and ctx.storeValue() are true, then the stored value will never be used. But relating to common usage of CacheObject.value method it's seems that usual value of {cpy} is false, so the problem is virtual.

I'm wondering that nothing of above was written in Javadoc.

Going further, I'm answering myself when the copyOnGet() and storeValue() should be true or false?

Definitly they should depend upon user preference CacheConfiguration.isCopyOnRead(): copyOnGet() == isCopyOnRead(); storeValue() != isCopyOnRead().

Also we should take in account how indexing are working. Whenever indexing is enabled on the cache, some number of GridH2ValueCacheObject are present (up to number of cache entries). There is the method GridH2ValueCacheObject.compareSecure(Value v, CompareMode mode) which compares the two CacheObject or its serialized user objects, depending of the CacheObject.isPlatformType() value. So, because compareSecure() method is time critical, we should set storeValue() to true, when indexing is enabled AND CacheObject.isPlatformType() is true for all entries in the cache, to avoid high CPU overhead upon deserializing user object on each compareSecure() invocation.

Actual CacheObject type stored in the cache depends on the IgniteConfiguration.getMarshaller() value configured. The GridQueryProcessor.isEnabled(CacheConfiguration ccfg) doesn't take into account what marshaller type is used, but only whether indexing is enabled. It should be fixed.

There is another condition IgniteConfiguration.isPeerClassLoadingEnabled() is used to determine CacheObjectContext.storeValue(). It is still being discovered.

> Deserialized value is cached if queries are enabled
> ---------------------------------------------------
>
>                 Key: IGNITE-4293
>                 URL: https://issues.apache.org/jira/browse/IGNITE-4293
>             Project: Ignite
>          Issue Type: Bug
>          Components: cache
>    Affects Versions: 1.7
>            Reporter: Valentin Kulichenko
>            Assignee: Alexandr Kuramshin
>            Priority: Critical
>
> Here is the problematic piece of code in {{IgniteCacheObjectProcessorImpl}}:
> {code}
> boolean storeVal = ctx.config().isPeerClassLoadingEnabled() ||
>     GridQueryProcessor.isEnabled(ccfg) ||
>     !ccfg.isCopyOnRead();
> {code}
> This flag is set to true if queries are enabled even when binary marshaller is used (this condition makes sense to other marshallers though). It is then use in {{BinaryObjectImpl.deserializeValue}}:
> {code}
> if (coCtx != null && coCtx.storeValue())
>     obj = obj0;
> {code}
> As a result, memory consumption doubles.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)