Review request for JDK-8136700: Make sure Context.anonymousHostClasses doesn't grow unbounded

Hannes Wallnoefer hannes.wallnoefer at oracle.com
Fri Sep 18 14:13:52 UTC 2015


Am 2015-09-18 um 15:59 schrieb Sundararajan Athijegannathan:
> I presume it is for code clarity sake? That is a private method in 
> that class called only from the other synchronized method. How about 
> just adding an assert using Thread.holdsLock?

Well it's mostly as a guard for potential changes down the road. Nothing 
says "this method needs to be synchronized" better than just making it 
synchronized IMO. :)

Hannes

> -Sundar
>
> On 9/18/2015 7:14 PM, Hannes Wallnoefer wrote:
>> Shouldn't the methods modifying the map also be synchronized, even if 
>> we know they are (currently) only called from a synchronized method?
>>
>> Otherwise looks good.
>>
>> Hannes
>>
>> Am 2015-09-18 um 14:05 schrieb Sundararajan Athijegannathan:
>>> +1
>>>
>>> On 9/18/2015 3:24 PM, Attila Szegedi wrote:
>>>>> On Sep 18, 2015, at 6:22 AM, Sundararajan Athijegannathan 
>>>>> <sundararajan.athijegannathan at oracle.com> wrote:
>>>>>
>>>>> * Context.compile is synchronized and so there is no need for 
>>>>> ConcurrentHashMap + concurrency related comment.
>>>> You’re right, I removed them. I updated webrev in-place.
>>>>
>>>>> * Map.remove(Object, Object) is used which is since jdk 1.8. If we 
>>>>> have to backport this change to jdk8u (where jdk7 is used as 
>>>>> JAVA_HOME to build/test), we've to take care of this.
>>>> If we backport, this will show up as a compile error, so we’ll 
>>>> adjust then. I’d rather run another round of a short review then 
>>>> than have a less efficient construct in 9 codebase.
>>>>
>>>> Thanks,
>>>>    Attila.
>>>>
>>>>> -Sundar
>>>>>
>>>>> On 9/17/2015 9:07 PM, Attila Szegedi wrote:
>>>>>> Indeed, that’s correct. A new webrev is available for review at 
>>>>>> <http://cr.openjdk.java.net/~attila/8136700/webrev.01.jdk9> that 
>>>>>> manually handles removal of cleared entries.
>>>>>>
>>>>>> Attila.
>>>>>>
>>>>>>> On Sep 17, 2015, at 4:29 PM, Michael Haupt 
>>>>>>> <michael.haupt at oracle.com> wrote:
>>>>>>>
>>>>>>> Hi again,
>>>>>>>
>>>>>>> no sorry; I have to revoke this as I forgot weak sets aren't 
>>>>>>> weak in the way they have to be in this case.
>>>>>>>
>>>>>>> Best,
>>>>>>>
>>>>>>> Michael
>>>>>>>
>>>>>>>> Am 17.09.2015 um 16:18 schrieb Michael Haupt 
>>>>>>>> <michael.haupt at oracle.com>:
>>>>>>>>
>>>>>>>> Hi Attila,
>>>>>>>>
>>>>>>>> lower-case thumbs up!
>>>>>>>>
>>>>>>>> Best,
>>>>>>>>
>>>>>>>> Michael
>>>>>>>>
>>>>>>>>> Am 17.09.2015 um 16:14 schrieb Attila Szegedi 
>>>>>>>>> <attila.szegedi at oracle.com>:
>>>>>>>>>
>>>>>>>>> Please review JDK-8136700 "Make sure 
>>>>>>>>> Context.anonymousHostClasses doesn't grow unbounded" at 
>>>>>>>>> <http://cr.openjdk.java.net/~attila/8136700/webrev.jdk9> for 
>>>>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8136700>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Attila.
>>>>>>> -- 
>>>>>>>
>>>>>>> <http://www.oracle.com/>
>>>>>>> Dr. Michael Haupt | Principal Member of Technical Staff
>>>>>>> Phone: +49 331 200 7277 | Fax: +49 331 200 7561
>>>>>>> Oracle Java Platform Group | LangTools Team | Nashorn
>>>>>>> Oracle Deutschland B.V. & Co. KG, Schiffbauergasse 14 | 14467 
>>>>>>> Potsdam, Germany
>>>>>>> <http://www.oracle.com/commitment>    Oracle is committed to 
>>>>>>> developing practices and products that help protect the environment
>>>>>>>
>>>
>>
>



More information about the nashorn-dev mailing list