Bug #24351 SingleByteCharsetConverter high contention on getInstance()
Submitted: 16 Nov 2006 2:53 Modified: 27 Feb 2007 13:56
Reporter: Quartz 12h Email Updates:
Status: Closed Impact on me:
None 
Category:Connector / J Severity:S5 (Performance)
Version:3.1.10 OS:
Assigned to: CPU Architecture:Any

[16 Nov 2006 2:53] Quartz 12h
Description:
This applies to many driver versions, from the 3.1.10 I am using, to 5.0.0 beta I looked into, and probably more.

There is a high level of contention in the SingleByteCharsetConverter.getInstance() as observed with a jvm profiler.

How to repeat:
I have about 50 threads doing inserts non-stop, and 80% of the causes of blocked thread is this.

Suggested fix:
Simple, use a static threadlocal map in the class to cache the converters in the thread:

In the class:

	private static final ThreadLocal CONVERTER_MAP_TL = new ThreadLocal() {
		protected Object initialValue() {
			return new HashMap();
		}
	};

Remove the 'synchronized' from the getInstance and do this:

	public static SingleByteCharsetConverter getInstance(
			String encodingName, Connection conn)
			throws UnsupportedEncodingException, SQLException {
		
		Map tlMap = (Map)CONVERTER_MAP_TL.get();
		SingleByteCharsetConverter instance = (SingleByteCharsetConverter) tlMap.get(encodingName);
		if(instance==null) {
			synchronized(SingleByteCharsetConverter.class) {
				instance = (SingleByteCharsetConverter) CONVERTER_MAP.get(encodingName);
				if (instance == null) {
					instance = initCharset(encodingName);
				}
			}
			tlMap.put(encodingName, instance);
		}
		return instance;
	}

After that, the hotspot of blocked threads COMPLETELY disappeared from the radar of course.
[16 Nov 2006 12:53] Quartz 12h
Of course, this ThreadLocal means jdk 1.2+ compatibility. Otherwise, you would have to cache those converters in a map in the Connection, given the connection is supposed to be used by only one thread at a time. Notice that this converter instance caching only caches the references, it doesn't create new converters per connection nor thread.
[28 Nov 2006 16:27] Quartz 12h
I can see it is assigned and being analysed.
Any feedback, and ETA if possible, would be appreciated.
Thanks.
[1 Dec 2006 9:18] Tonci Grgin
Hi and thanks for your problem report. Sorry for the delay in processing. Can you please try with more recent version of connector/J, as the one you're using is rather old, and get back to me with results.
[1 Dec 2006 23:07] Quartz 12h
modified version

Attachment: SingleByteCharsetConverter.java (application/octet-stream, text), 7.91 KiB.

[2 Jan 2007 0:00] Bugs System
No feedback was provided for this bug for over a month, so it is
being suspended automatically. If you are able to provide the
information that was originally requested, please do so and change
the status of the bug back to "Open".
[2 Jan 2007 17:27] Quartz 12h
The performance issue exists with latest 5.0.4.
Note that the file hasn't changed since 3.1.10.

A patch was attached. I don't understand why this issue timed out on feedback...
[2 Jan 2007 17:40] Tonci Grgin
Hi. You probably didn't change the status... Anyway I am asking for consult on this.
[4 Jan 2007 18:19] Mark Matthews
Hi,

You don't happen to have a callstack that shows where this hotspot takes place? Code inspection shows that these should be cached _per_connection_, so if you're not letting connections have very short lifetimes, you shouldn't see this contention, unless we're missing a code path that your application's use case exposes.
[4 Jan 2007 21:34] Quartz 12h
at Statement.executeBatch(688)
at Statement.executeUpdate(929)
at Connection.execSQL
at Connection.execSQL(2972)
at MysqlIO.sqlQueryDirect(1637)
at ByteArrayBuffer.writeStringNoNull(544)
at StringUtils.getBytes(590)
[4 Jan 2007 22:37] Quartz 12h
Previous stack trace is with 3.1.10.
Ok, you guys have cached the reference in the connection; the surrounding code has changed but not the SingleByteCharsetConverter. That's why I was fooled. Ok.

But as I reran the profiler, I see a whole new blocking point; ResultSet and generated keys stuff I never saw before, taking the 1st place for contention...

Statement.executeBatch(900)
Statement.getBatchedGeneratedKeys(2284)
Statement.getGeneratedKeysInternal()
ResultSet.<init>(338)
java.util.GregorianCalendal.<init>(562)
java.util.GregorianCalendal.<init>(574)
java.util.Calendal.<init>(897)
java.util.Calendal.setWeekCountData()
java.util.Hashtable.get()

Of course, the hashtable read is synchronized and a static member. Cute.

Have fun!
[4 Jan 2007 22:56] Mark Matthews
Yeah, fun. Core APIs not being designed for scalability bite us again (we went through this with java.util.Date and friends being statically synchronized all over the place).

I'll have to look and see what happens if we lazily instantiate the calendar only when results touch something date-time related. Of course that complicates synchronization.
[13 Feb 2007 20:53] Mark Matthews
In 5.0.5 and later (soon to be released, nightly snapshots are available at http://download.mysql.com/snapshots.php#connector-j), we now lazily instantiate calendars and timezones when needed, hopefully this helps (it no longer shows up as a contention issue in our internal testing).
[27 Feb 2007 13:56] MC Brown
A note have been added to the 5.0.5 changelog.