[ https://issues.apache.org/jira/browse/SLING-9769?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Julian Sedding updated SLING-9769: ---------------------------------- Description: SLING-4216 introduced a limit to the number of cached vanity paths. If the cache size is exceeded, it falls back to a repository search, which is relatively costly. So in order to avoid most of the costly calls, a bloom filter was introduced to answer the question whether a search is likely to yield a result. The bloom filter is by default nearly 1MB in size (without Java overhead) and it is persisted in a file and a {{java.util.Timer}} was introduced "for persisting the bloom filter every minute". The memory leak is that the {{Timer}} is not cancelled when {{MapEntries#dispose}} is called, and thus the timer's thread holds on to a reference of the {{MapEntries}} instance. Now the severity of this issue is low for two reasons: 1. MapEntries is a singleton in a normal Sling deployment. 2. A bug causes the {{TimerTask}} to be executed only once after 60 seconds and not repeatedly. I.e. the thread disappeared after ~1 minute. I discovered the memory leak when running unit tests that use Sling-Mock. During testing a new {{ResourceResolverFactory}} instance, together with its associated {{MapEntries}} is created for every test method. In a module with ~700 test methods that lead to an OutOfMemoryError}} with 1GB max heap size. Fixing the memory leak allowed the same module to run with only 96MB max heap. I intend to also fix the {{Timer}} to be scheduled to run once every minute. This intention is indicated by a comment in the code. The use of a non-volatile boolean field also looks like it might be prone to concurrency/visibility issues and may be better replaced by an {{AtomicBoolean}}. cc [~asanso] [~sseifert] do you think it would make sense for Sling Mock to set {{resource.resolver.vanitypath.bloomfilter.maxBytes = 0}} by default? was: SLING-4216 introduced a limit to the number of cached vanity paths. If the cache size is exceeded, it falls back to a repository search, which is relatively costly. So in order to avoid most of the costly calls, a bloom filter was introduced to answer the question whether a search is likely to yield a result. The bloom filter is by default nearly 1MB in size (without Java overhead) and it is persisted in a file and a {{java.util.Timer}} was introduced "for persisting the bloom filter every minute". The memory leak is that the {{Timer}} is not cancelled when {{MapEntries#dispose}} is called, and thus the timer's thread holds on to a reference of the {{MapEntries}} instance. Now the severity of this issue is low for two reasons: 1. MapEntries is a singleton in a normal Sling deployment. 2. A bug causes the {{TimerTask}} to be executed only once after 60 seconds and not repeatedly. I.e. the thread disappeared after ~1 minute. I discovered the memory leak when running unit tests that use Sling-Mock. During testing a new {{ResourceResolverFactory}} instance, together with its associated {{MapEntries}} is created for every test method. In a module with ~700 test methods that lead to an OutOfMemoryError}} with 1GB max heap size. Fixing the memory leak allowed the same module to run with only 96MB max heap. I intend to also fix the {{Timer}} to be scheduled to run once every minute. This intention is indicated by a comment in the code. The use of a non-volatile boolean field also looks like it might be prone to concurrency/visibility issues and may be better replaced by an {{AtomicBoolean}}. cc [~asanso] > Minor memory leak in MapEntries class > ------------------------------------- > > Key: SLING-9769 > URL: https://issues.apache.org/jira/browse/SLING-9769 > Project: Sling > Issue Type: Bug > Components: ResourceResolver > Affects Versions: Resource Resolver 1.1.10 > Reporter: Julian Sedding > Assignee: Julian Sedding > Priority: Minor > Time Spent: 20m > Remaining Estimate: 0h > > SLING-4216 introduced a limit to the number of cached vanity paths. If the > cache size is exceeded, it falls back to a repository search, which is > relatively costly. So in order to avoid most of the costly calls, a bloom > filter was introduced to answer the question whether a search is likely to > yield a result. > The bloom filter is by default nearly 1MB in size (without Java overhead) and > it is persisted in a file and a {{java.util.Timer}} was introduced "for > persisting the bloom filter every minute". > The memory leak is that the {{Timer}} is not cancelled when > {{MapEntries#dispose}} is called, and thus the timer's thread holds on to a > reference of the {{MapEntries}} instance. Now the severity of this issue is > low for two reasons: > 1. MapEntries is a singleton in a normal Sling deployment. > 2. A bug causes the {{TimerTask}} to be executed only once after 60 seconds > and not repeatedly. I.e. the thread disappeared after ~1 minute. > I discovered the memory leak when running unit tests that use Sling-Mock. > During testing a new {{ResourceResolverFactory}} instance, together with its > associated {{MapEntries}} is created for every test method. In a module with > ~700 test methods that lead to an OutOfMemoryError}} with 1GB max heap size. > Fixing the memory leak allowed the same module to run with only 96MB max heap. > I intend to also fix the {{Timer}} to be scheduled to run once every minute. > This intention is indicated by a comment in the code. > The use of a non-volatile boolean field also looks like it might be prone to > concurrency/visibility issues and may be better replaced by an > {{AtomicBoolean}}. > cc [~asanso] > [~sseifert] do you think it would make sense for Sling Mock to set > {{resource.resolver.vanitypath.bloomfilter.maxBytes = 0}} by default? -- This message was sent by Atlassian Jira (v8.3.4#803005)