[ https://issues.apache.org/jira/browse/NETBEANS-96?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16240987#comment-16240987 ]
ASF GitHub Bot commented on NETBEANS-96: ---------------------------------------- lbruun commented on a change in pull request #161: [NETBEANS-96] New PAC Script evaluation environment URL: https://github.com/apache/incubator-netbeans/pull/161#discussion_r149217448 ########## File path: core.network/src/org/netbeans/core/network/utils/SimpleObjCache.java ########## @@ -0,0 +1,136 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.netbeans.core.network.utils; + +import java.util.Comparator; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicLong; + +/** + * Simple cache with a max capacity. Encapsulates {@link java.util.Map Map}. + * + * <p> + * Eviction is based on usage. Every time a value is retrieved from the cache + * its 'usage timestamp' will change. When the cache is full, the value with the + * oldest usage timestamp will be evicted. + * + * <p> + * This class is thread-safe and is very efficient for read operations + * ({@code get}). The write operation ({@code put}) will - if the cache is full + * - have performance which degrades linearly with the size of the cache as all + * elements will have to be inspected to find the eviction candidate. For this + * reason, the class is best suited for smaller caches (say less than 1000 + * elements). + * + * + * @author lbruun + * + * @param <K> key + * @param <V> value + */ +public class SimpleObjCache<K,V> { + + private final int maxSize; + private final ConcurrentHashMap<K, ValueWrapper> map; + + /** + * Constructs a new cache with {@code maxSize} capacity. When the + * cache is full, the value used/created the furthest in the past will + * be evicted to make room for a new element. + * + * @param maxSize capacity - any value larger than zero + */ + public SimpleObjCache(int maxSize) { + + if (maxSize <= 0) { + throw new IllegalArgumentException("Cache max size cannot be zero or less"); + } + this.maxSize = maxSize; + this.map = new ConcurrentHashMap<>(); + } + + /** + * Puts a value into the cache mapped to the specified key. + * + * @param key key with which the specified value is to be associated + * @param value value to be associated with the specified key + * @return the previous value associated with key, or null if there + * was no mapping for the key + */ + public V put(K key, V value) { + if (map.size() >= maxSize) { + map.remove(findEvictionCandidate()); + } + ValueWrapper existingValue = map.put(key, new ValueWrapper(value)); + if (existingValue != null) { + return existingValue.value; + } else { + return null; + } + } + + /** + * Returns the value to which the specified key is mapped, or {@code null} + * if this cache contains no mapping for the key. + * + * @param key the key whose associated value is to be returned + * @return the value to which the specified key is mapped, or {@code null} + * if this map contains no mapping for the key + */ + public V get(K key) { + ValueWrapper wrapper = map.get(key); + if (wrapper != null) { + wrapper.lastUsed.set(System.currentTimeMillis()); + return wrapper.value; + } + return null; + } + + /** + * Gets the current number of elements in the cache. + * @return + */ + public int getCacheSize() { + return map.size(); + } + + /** + * Removes all elements from the cache. There is little reason to use + * this method as the cache will do its own house keeping. + */ + public void clear() { + map.clear(); + } + + private K findEvictionCandidate() { + // Finds minimum of all timestamps of all elements in the cache. + return map.entrySet().stream() + .min(Comparator.comparingLong( e -> e.getValue().lastUsed.get())) + .get().getKey(); Review comment: There's actually still a point to having the `synchronized` there. It is not because of thread-safety because the code - even without the `synchronized` - is free from concurrency exceptions, However, it protects against the situation when multiple threads simultaneously arrive at the conclusion that eviction is required, so they all perform an evict, thereby essentially evicting _more_ elements than is strictly needed. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > New PAC Script evaluator > ------------------------ > > Key: NETBEANS-96 > URL: https://issues.apache.org/jira/browse/NETBEANS-96 > Project: NetBeans > Issue Type: Improvement > Reporter: lbruun > Labels: pull-request-available > > The current [PAC script|https://en.wikipedia.org/wiki/Proxy_auto-config] > evaluator (in {{core.network}}) was developed pre-Nashorn and has a few > problems: > * It simply fails with Nashorn - but not with Rhino - if the downloaded > script uses {{isInNet()}}. This was reported in [Bug > 245116|https://netbeans.org/bugzilla/show_bug.cgi?id=245116]. It fails > silently in this case and defaults to no proxy. The user will never know the > reason - not even by looking in the message log - that there was an error. > * It doesn't implement two mandatory JavaScript helper methods, > {{dnsResolve()}} and {{myIpAddress()}}. This is a known issue. This causes > many PAC scripts to silently fail. > * It doesn't implement Microsoft's IPv6-aware additions to the PAC standard. > This is a problem in MS shops because they will have designed their PAC > script to be compatible with MS IE and MS Edge (which unsurprisingly support > these functions .. as do Chrome). > * It uses a small JavaScript helper, {{nsProxyAutoConfig.js}}, which uses a > license which is not compatible with Apache. This is described in NETBEANS-4. > * Isn't executing the downloaded PAC script in a sandboxed environment. (The > PAC script should be treated as hostile because the download may have been > spoofed. Browsers indeed treat the PAC script as hostile and so should > NetBeans). > Pull Request with a new implementation is on its way. -- This message was sent by Atlassian JIRA (v6.4.14#64029)