MySQL Bugs: #24351: SingleByteCharsetConverter high contention on getInstance()

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

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.

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.

I can see it is assigned and being analysed.
Any feedback, and ETA if possible, would be appreciated.
Thanks.

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.

modified version

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

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".

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...

Hi. You probably didn't change the status... Anyway I am asking for consult on this.

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.

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)

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!

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.

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).

A note have been added to the 5.0.5 changelog.