[
https://issues.apache.org/jira/browse/SLING-8946?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17044413#comment-17044413
]
Dirk Rudolph edited comment on SLING-8946 at 2/25/20 12:56 PM:
---------------------------------------------------------------
For now implementing SLING-9077 seems the easiest to progress on this for now.
I tested the benchmark provided by [~sonagupt] with varying numbers of
ResourceProviders (100,200,400,800 and 1600) and compared the results of the
test with and without my proposed changes for SLING-9077. In neither of both
cases the CPU time increases linear with the number of ResourceProviders
unfortunately. Though with SLING-9077 the runtime of adding 1600
ResourceProviders compared to add only 100 ResourceProviders improves from
*~80x* to *~5x* (based on median and assuming linear growth). See
[https://docs.google.com/spreadsheets/d/1r70aU6p5sCrGZibh43ANSDfXk9aCo1VfinKbNNYPJLI/edit#gid=1573960874]
for a details analysis
[~sonagupt] can you test if you ITs complete when using the changes to
org.apache.sling.api bundle in SLING-9077?
I know this is not a proper solution (but maybe a sufficient one). For a proper
solution I think we would need to make the PathSet used for the exclude Paths
mutable so that overlapping ResourceProvider's can be added/removed as the RP
actually appears/disappears. Now this sounds easier as it is but especially in
the BasicObservationReporter's constructor some optimisation is applied when
collecting the ObservationConfigurations. Also that would mean that the
JcrResourceProvider cannot register it's listeners on update() with the
excludePaths set anymore (not implemented anyway in Oak afaik).
[~cziegeler] can you remember why BasicObservationReporter l101 ff [1] is
necessary? As far as I can see the condition on the providerPath should be
quite restrictive already, for example:
{quote}providerPath = /
excludedPaths =
[/libs/sling/Folder/s7publish.json.POST.servlet,/libs/sling/distribution/services/agents,...]
info.paths = [/content,/var]
{quote}
Can you give an example what this code was supposed to handle?
[1]
[https://github.com/apache/sling-org-apache-sling-resourceresolver/blob/master/src/main/java/org/apache/sling/resourceresolver/impl/observation/BasicObservationReporter.java#L101]
was (Author: diru):
For now implementing SLING-9077 seems the easiest to progress on this for now.
I tested the benchmark provided by [~sonagupt] with varying numbers of
ResourceProviders (100,200,400,800 and 1600) and compared the results of the
test with and without my proposed changes for SLING-9077. In neither of both
cases the CPU time increases linear with the number of ResourceProviders
unfortunately. Though with SLING-9077 the runtime of adding 1600
ResourceProviders compared to add only 100 ResourceProviders improves from
*~80x* to *~5x* (based on median and assuming linear growth)
[~sonagupt] can you test if you ITs complete when using the changes to
org.apache.sling.api bundle in SLING-9077?
I know this is not a proper solution (but maybe a sufficient one). For a proper
solution I think we would need to make the PathSet used for the exclude Paths
mutable so that overlapping ResourceProvider's can be added/removed as the RP
actually appears/disappears. Now this sounds easier as it is but especially in
the BasicObservationReporter's constructor some optimisation is applied when
collecting the ObservationConfigurations. Also that would mean that the
JcrResourceProvider cannot register it's listeners on update() with the
excludePaths set anymore (not implemented anyway in Oak afaik).
[~cziegeler] can you remember why BasicObservationReporter l101 ff [1] is
necessary? As far as I can see the condition on the providerPath should be
quite restrictive already, for example:
{quote}providerPath = /
excludedPaths =
[/libs/sling/Folder/s7publish.json.POST.servlet,/libs/sling/distribution/services/agents,...]
info.paths = [/content,/var]
{quote}
Can you give an example what this code was supposed to handle?
[1]
[https://github.com/apache/sling-org-apache-sling-resourceresolver/blob/master/src/main/java/org/apache/sling/resourceresolver/impl/observation/BasicObservationReporter.java#L101]
> Non-deterministic shadowing of resource observation
> ---------------------------------------------------
>
> Key: SLING-8946
> URL: https://issues.apache.org/jira/browse/SLING-8946
> Project: Sling
> Issue Type: Bug
> Components: ResourceResolver
> Affects Versions: Resource Resolver 1.5.34
> Reporter: Dirk Rudolph
> Assignee: Robert Munteanu
> Priority: Critical
> Fix For: Resource Resolver 1.6.18
>
> Attachments: PerformanceScript.sh
>
> Time Spent: 1.5h
> Remaining Estimate: 0h
>
> The BasicObservationReporter implements shadowing of events being propagated
> per ResourceProvider instance [1]. Assuming we do have 2 ResourceProviders
> registered, where the one shadows the other like this:
> - RP A registered on /
> - RP B registered on /path
> then currently the excludes given to the BasicObservationReporter are
> different depending on either RP A or RP B gets registered first:
> - RP A before RP B => excludes are empty
> - RP B before RP A => excludes contain /path
> This is because only the newly registered RP gets its ProviderContext updated
> [2]
> Same applies if RP B is registered before RP A and gets unregistered. In that
> case the ObservationReporter of RP A stays untouched.
> [1]
> [https://github.com/apache/sling-org-apache-sling-resourceresolver/blob/master/src/main/java/org/apache/sling/resourceresolver/impl/observation/BasicObservationReporter.java#L102]
> [2]
> [https://github.com/apache/sling-org-apache-sling-resourceresolver/blob/master/src/main/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderTracker.java#L358]
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
