maven-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [maven-resolver] tawek commented on a change in pull request #22: Introduce caching for tracking files.
Date Mon, 01 Apr 2019 21:58:47 GMT
tawek commented on a change in pull request #22: Introduce caching for tracking files.
URL: https://github.com/apache/maven-resolver/pull/22#discussion_r271065809
 
 

 ##########
 File path: maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/TrackingFileManager.java
 ##########
 @@ -131,52 +207,145 @@ public Properties update( File file, Map<String, String> updates
)
                 raf.seek( 0 );
                 raf.write( stream.toByteArray() );
                 raf.setLength( raf.getFilePointer() );
+
+                this.entryTs = System.currentTimeMillis();
+                this.lastModifiedTs = file.lastModified();
             }
             catch ( IOException e )
             {
                 LOGGER.warn( "Failed to write tracking file {}", file, e );
             }
             finally
             {
-                release( lock, file );
-                close( raf, file );
+                release( lock );
+                close( raf );
             }
-        }
 
-        return props;
-    }
+            return props;
+        }
 
-    private void release( FileLock lock, File file )
-    {
-        if ( lock != null )
+        private void release( FileLock lock )
         {
-            try
+            if ( lock != null )
             {
-                lock.release();
+                try
+                {
+                    lock.release();
+                }
+                catch ( IOException e )
+                {
+                    LOGGER.warn( "Error releasing lock for tracking file {}", file, e );
+                }
             }
-            catch ( IOException e )
+        }
+
+        private void close( Closeable closeable )
+        {
+            if ( closeable != null )
             {
-                LOGGER.warn( "Error releasing lock for tracking file {}", file, e );
+                try
+                {
+                    closeable.close();
+                }
+                catch ( IOException e )
+                {
+                    LOGGER.warn( "Error closing tracking file {}", file, e );
+                }
             }
         }
-    }
 
-    private void close( Closeable closeable, File file )
-    {
-        if ( closeable != null )
+        private FileLock lock( FileChannel channel, long size, boolean shared ) throws IOException
         {
-            try
+            FileLock lock = null;
+
+            for ( int attempts = 8; attempts >= 0; attempts-- )
             {
-                closeable.close();
+                try
+                {
+                    lock = channel.lock( 0, size, shared );
+                    break;
+                }
+                catch ( OverlappingFileLockException e )
+                {
+                    if ( attempts <= 0 )
+                    {
+                        throw (IOException) new IOException().initCause( e );
+                    }
+                    try
+                    {
+                        Thread.sleep( 50L );
+                    }
+                    catch ( InterruptedException e1 )
+                    {
+                        Thread.currentThread().interrupt();
+                    }
+                }
             }
-            catch ( IOException e )
+
+            if ( lock == null )
             {
-                LOGGER.warn( "Error closing tracking file {}", file, e );
+                throw new IOException( "Could not lock file" );
             }
+
+            return lock;
+        }
+
+    }
+
+    /**
+     * Canonicalized files by their path (the same canonicalized file may be registered under
different paths). This
+     * cache is especially useful on Windows platform where canonicalization is relatively
expensive.
+     */
+    private static ConcurrentHashMap<String, File> canonicalizedCache = new ConcurrentHashMap<>();
 
 Review comment:
   Glad to here there is some interest in merging either this or #29. 
   
   As to split "this and canonicalization" - I understand that we need to have a different
PR on canonicalization cache.
   
   As to "I need JIRA" - as there is already MRESOLVER-68 so I may hijack it or create two
new JIRAs tickets (assuming there is a real need to split). Please advise.
   
   For the weakly reachable keys in the canonicalization cache hash map I understand the concern
for memory. But since WeakHashMap expiry is based on key object identity it effectively means
that caching for canonicalization may not work unless there is String deduplication enabled
on JVM. Please see the javadoc for WeakHashMap. For File its even worse since those handles
are not deduplicated in any way as far as I know and the caches would be flushed clean on
gc (probably depends on collector impl and finalizer threads/reference queues).
   
   There is already a canonicalization cache in JVM for Windows (!) - see 'ExpiringCache.java'
it is just limited to 200 entries FIFO and 30s. I've found it to be ineffective - too small
and too time-limited. Therefore I haven't put a time nor size constraint as indicated in TODO
but I agree there should be some size bound just in case of some runaway build ...
   
   My choice therefore would be to make LRU cache with some 10000 entries bound (not to be
reached ever - just in case of runaway programs) with TTL of couple of minutes (threshold
at which a human will no longer be willing to wait for m2e and will just make the projects
smaller :) )

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message