You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-dev@db.apache.org by "Knut Anders Hatlen (JIRA)" <ji...@apache.org> on 2010/01/12 15:09:54 UTC
[jira] Created: (DERBY-4512) Avoid unnecessary lookup in
transaction table when adding transaction
Avoid unnecessary lookup in transaction table when adding transaction
---------------------------------------------------------------------
Key: DERBY-4512
URL: https://issues.apache.org/jira/browse/DERBY-4512
Project: Derby
Issue Type: Improvement
Components: Store
Affects Versions: 10.6.0.0
Reporter: Knut Anders Hatlen
Assignee: Knut Anders Hatlen
Priority: Minor
TransactionTable.add() first checks if the Hashtable trans contains a transaction with the same id as the one being added. If it does, add() does nothing. If there is no such transaction, a new TransactionTableEntry is created and put into the Hashtable.
I believe that TransactionTable.add() is never called on a transaction that has already been added to the table. If this is the case, there's no point in checking the Hashtable first. Instead, we could just create a new TransactionTableEntry and add it unconditionally. This would reduce the number of (synchronized) Hashtable calls and could improve the performance in scenarios like the one described in DERBY-3092.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Closed: (DERBY-4512) Avoid unnecessary lookup in transaction
table when adding transaction
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4512?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Knut Anders Hatlen closed DERBY-4512.
-------------------------------------
Resolution: Fixed
Fix Version/s: 10.6.0.0
Issue & fix info: (was: [Patch Available])
Committed revision 900714.
> Avoid unnecessary lookup in transaction table when adding transaction
> ---------------------------------------------------------------------
>
> Key: DERBY-4512
> URL: https://issues.apache.org/jira/browse/DERBY-4512
> Project: Derby
> Issue Type: Improvement
> Components: Store
> Affects Versions: 10.6.0.0
> Reporter: Knut Anders Hatlen
> Assignee: Knut Anders Hatlen
> Priority: Minor
> Fix For: 10.6.0.0
>
> Attachments: assert.diff, txtab-noblanks.diff, txtab.diff
>
>
> TransactionTable.add() first checks if the Hashtable trans contains a transaction with the same id as the one being added. If it does, add() does nothing. If there is no such transaction, a new TransactionTableEntry is created and put into the Hashtable.
> I believe that TransactionTable.add() is never called on a transaction that has already been added to the table. If this is the case, there's no point in checking the Hashtable first. Instead, we could just create a new TransactionTableEntry and add it unconditionally. This would reduce the number of (synchronized) Hashtable calls and could improve the performance in scenarios like the one described in DERBY-3092.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4512) Avoid unnecessary lookup in
transaction table when adding transaction
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4512?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12799243#action_12799243 ]
Knut Anders Hatlen commented on DERBY-4512:
-------------------------------------------
Thanks Bryan. I was thinking that we could check the return value from Hashtable.put(), which should be null if there was no previous mapping registered for that transaction id.
I tried such a change with the d2911perf client used in DERBY-3092 on a 32-way system. With 32 threads, trunk performed 158K tx/s and the patched version 182K tx/s, which means about 15% improvement.
Still, this patch is far less effective than Dyre's patch that replaces the Hashtable with a ConcurrentHashMap. With that patch, I saw 230K tx/s. It is however possible to combine those two patches and get an additional 3-4% increase in the throughput on top of that, it seems, so I think it's going to be beneficial regardless of how/if we end up addressing DERBY-3092.
Another thing I've noticed with TransactionTable.add() is that the synchronization on TransactionTable.this is probably not needed (will file another JIRA issue for that). My understanding is that synchronization on TransactionTable.this is used to protect transaction state changes from read to update. No such transition happens within that synchronized block, so the (implicit) Hashtable synchronization should be enough.
> Avoid unnecessary lookup in transaction table when adding transaction
> ---------------------------------------------------------------------
>
> Key: DERBY-4512
> URL: https://issues.apache.org/jira/browse/DERBY-4512
> Project: Derby
> Issue Type: Improvement
> Components: Store
> Affects Versions: 10.6.0.0
> Reporter: Knut Anders Hatlen
> Assignee: Knut Anders Hatlen
> Priority: Minor
> Attachments: assert.diff
>
>
> TransactionTable.add() first checks if the Hashtable trans contains a transaction with the same id as the one being added. If it does, add() does nothing. If there is no such transaction, a new TransactionTableEntry is created and put into the Hashtable.
> I believe that TransactionTable.add() is never called on a transaction that has already been added to the table. If this is the case, there's no point in checking the Hashtable first. Instead, we could just create a new TransactionTableEntry and add it unconditionally. This would reduce the number of (synchronized) Hashtable calls and could improve the performance in scenarios like the one described in DERBY-3092.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4512) Avoid unnecessary lookup in
transaction table when adding transaction
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4512?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Knut Anders Hatlen updated DERBY-4512:
--------------------------------------
Issue & fix info: [Patch Available]
> Avoid unnecessary lookup in transaction table when adding transaction
> ---------------------------------------------------------------------
>
> Key: DERBY-4512
> URL: https://issues.apache.org/jira/browse/DERBY-4512
> Project: Derby
> Issue Type: Improvement
> Components: Store
> Affects Versions: 10.6.0.0
> Reporter: Knut Anders Hatlen
> Assignee: Knut Anders Hatlen
> Priority: Minor
> Attachments: assert.diff, txtab-noblanks.diff, txtab.diff
>
>
> TransactionTable.add() first checks if the Hashtable trans contains a transaction with the same id as the one being added. If it does, add() does nothing. If there is no such transaction, a new TransactionTableEntry is created and put into the Hashtable.
> I believe that TransactionTable.add() is never called on a transaction that has already been added to the table. If this is the case, there's no point in checking the Hashtable first. Instead, we could just create a new TransactionTableEntry and add it unconditionally. This would reduce the number of (synchronized) Hashtable calls and could improve the performance in scenarios like the one described in DERBY-3092.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (DERBY-4512) Avoid unnecessary lookup in
transaction table when adding transaction
Posted by "Bryan Pendleton (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4512?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12799219#action_12799219 ]
Bryan Pendleton commented on DERBY-4512:
----------------------------------------
+1 to removing the unnecessary check *and* leaving the SanityManager assertion as a verification.
> Avoid unnecessary lookup in transaction table when adding transaction
> ---------------------------------------------------------------------
>
> Key: DERBY-4512
> URL: https://issues.apache.org/jira/browse/DERBY-4512
> Project: Derby
> Issue Type: Improvement
> Components: Store
> Affects Versions: 10.6.0.0
> Reporter: Knut Anders Hatlen
> Assignee: Knut Anders Hatlen
> Priority: Minor
> Attachments: assert.diff
>
>
> TransactionTable.add() first checks if the Hashtable trans contains a transaction with the same id as the one being added. If it does, add() does nothing. If there is no such transaction, a new TransactionTableEntry is created and put into the Hashtable.
> I believe that TransactionTable.add() is never called on a transaction that has already been added to the table. If this is the case, there's no point in checking the Hashtable first. Instead, we could just create a new TransactionTableEntry and add it unconditionally. This would reduce the number of (synchronized) Hashtable calls and could improve the performance in scenarios like the one described in DERBY-3092.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4512) Avoid unnecessary lookup in
transaction table when adding transaction
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4512?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Knut Anders Hatlen updated DERBY-4512:
--------------------------------------
Attachment: txtab.diff
txtab-noblanks.diff
Here's a patch that makes the suggested change (txtab.diff). The patch looks somewhat messy because much code is moved up one indentation level because an if statement is removed. I've therefore also attached a patch generated with svn diff --extensions="-ub" which ignores whitespace changes, to make it easier to see exactly what has changed (this patch is called txtab-noblanks.diff).
The patch changes TransactionTable.add() in the following ways:
1) It does not call findTransactionEntry(id) first and checks the return value.
2) The return value from trans.put(id, ent) is captured, and an assert is added to verify that it is null (that is, the entry was not already there).
3) An assert that checked that the existing entry had the same exclusion as the one we attempt to add, is removed. This is because we now assert that there is no existing entry, which is a stricter condition.
TransactionTable.add() uses a mix of tabs and spaces for indentation. I chose to use spaces consistently for my changes.
All the regression tests ran cleanly with the patch.
> Avoid unnecessary lookup in transaction table when adding transaction
> ---------------------------------------------------------------------
>
> Key: DERBY-4512
> URL: https://issues.apache.org/jira/browse/DERBY-4512
> Project: Derby
> Issue Type: Improvement
> Components: Store
> Affects Versions: 10.6.0.0
> Reporter: Knut Anders Hatlen
> Assignee: Knut Anders Hatlen
> Priority: Minor
> Attachments: assert.diff, txtab-noblanks.diff, txtab.diff
>
>
> TransactionTable.add() first checks if the Hashtable trans contains a transaction with the same id as the one being added. If it does, add() does nothing. If there is no such transaction, a new TransactionTableEntry is created and put into the Hashtable.
> I believe that TransactionTable.add() is never called on a transaction that has already been added to the table. If this is the case, there's no point in checking the Hashtable first. Instead, we could just create a new TransactionTableEntry and add it unconditionally. This would reduce the number of (synchronized) Hashtable calls and could improve the performance in scenarios like the one described in DERBY-3092.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (DERBY-4512) Avoid unnecessary lookup in
transaction table when adding transaction
Posted by "Knut Anders Hatlen (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/DERBY-4512?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Knut Anders Hatlen updated DERBY-4512:
--------------------------------------
Attachment: assert.diff
None of the tests in suites.All or derbyall call TransactionTable.add() on a transaction that's already in the transaction table. I verified this by applying the attached patch, assert.diff, which raises an assert failure if a transaction with the same id is found, and running all the regression tests. All of the tests passed.
> Avoid unnecessary lookup in transaction table when adding transaction
> ---------------------------------------------------------------------
>
> Key: DERBY-4512
> URL: https://issues.apache.org/jira/browse/DERBY-4512
> Project: Derby
> Issue Type: Improvement
> Components: Store
> Affects Versions: 10.6.0.0
> Reporter: Knut Anders Hatlen
> Assignee: Knut Anders Hatlen
> Priority: Minor
> Attachments: assert.diff
>
>
> TransactionTable.add() first checks if the Hashtable trans contains a transaction with the same id as the one being added. If it does, add() does nothing. If there is no such transaction, a new TransactionTableEntry is created and put into the Hashtable.
> I believe that TransactionTable.add() is never called on a transaction that has already been added to the table. If this is the case, there's no point in checking the Hashtable first. Instead, we could just create a new TransactionTableEntry and add it unconditionally. This would reduce the number of (synchronized) Hashtable calls and could improve the performance in scenarios like the one described in DERBY-3092.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.