Merge lp:~pbeaman/akiban-persistit/optimize-value-get-2 into lp:akiban-persistit

Proposed by Peter Beaman
Status: Merged
Approved by: Nathan Williams
Approved revision: 397
Merged at revision: 397
Proposed branch: lp:~pbeaman/akiban-persistit/optimize-value-get-2
Merge into: lp:akiban-persistit
Diff against target: 417 lines (+41/-70)
14 files modified
src/main/java/com/persistit/Exchange.java (+3/-2)
src/main/java/com/persistit/Persistit.java (+2/-2)
src/main/java/com/persistit/Value.java (+1/-0)
src/test/java/com/persistit/BackupTaskTest.java (+0/-1)
src/test/java/com/persistit/Bug911849Test.java (+0/-1)
src/test/java/com/persistit/Bug915594Test.java (+0/-2)
src/test/java/com/persistit/Bug918909Test.java (+0/-2)
src/test/java/com/persistit/Bug932097Test.java (+0/-2)
src/test/java/com/persistit/Bug980292Test.java (+0/-2)
src/test/java/com/persistit/DumpTaskTest.java (+1/-1)
src/test/java/com/persistit/JournalManagerTest.java (+0/-1)
src/test/java/com/persistit/TreeTest2.java (+0/-1)
src/test/java/com/persistit/VolumeTest.java (+0/-1)
src/test/java/com/persistit/unit/ValueTest3.java (+34/-52)
To merge this branch: bzr merge lp:~pbeaman/akiban-persistit/optimize-value-get-2
Reviewer Review Type Date Requested Status
Nathan Williams Approve
Peter Beaman Needs Resubmitting
Review via email: mp+136061@code.launchpad.net

Description of the change

Add previously omitted call from directPut to preparePut.
Source code cleanup (small amount of formatting and removal of some obsolete imports)

To post a comment you must log in.
Revision history for this message
Nathan Williams (nwilliams) wrote :

Presumably trunk could hit an issue without this? Can we add a test for it?

Wasn't the "canonical reformatting part 3" suppose to avoid 175 lines of diff for 1 line of change? :)

review: Needs Fixing
397. By Peter Beaman

Add tests for directGet and directPut

Revision history for this message
Peter Beaman (pbeaman) wrote :

Done. I did a little cleanup in ValueTest3 too.

The 175 lines are mostly because in the support-spring-framework branch I forgot to run the Eclipse source->cleanup command which would have removed the obsolete imports. Source->Cleanup does more aggressive things than format, such as reorganizing the import lists, adding "final" qualifiers where possible, etc.

review: Needs Resubmitting
Revision history for this message
Nathan Williams (nwilliams) wrote :

New test looks good, thanks.

As I said when the big reformat branch went in, I'm not terribly thrilled if it isn't reproducible by everyone making changes or requires a manual step. The noise isn't helpful for reviews, even ones this small.

review: Approve
Revision history for this message
Peter Beaman (pbeaman) wrote :

I agree. But in this case it truly was due to an omission on my part which
I will try to avoid repeating. Overall I believe the format-related noise
has been minimal since we made that change, with this branch being the
exception.

On Mon, Nov 26, 2012 at 12:20 PM, Nathan Williams <email address hidden>wrote:

> Review: Approve
>
> New test looks good, thanks.
>
> As I said when the big reformat branch went in, I'm not terribly thrilled
> if it isn't reproducible by everyone making changes or requires a manual
> step. The noise isn't helpful for reviews, even ones this small.
> --
>
> https://code.launchpad.net/~pbeaman/akiban-persistit/optimize-value-get-2/+merge/136061
> You are the owner of lp:~pbeaman/akiban-persistit/optimize-value-get-2.
>

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/main/java/com/persistit/Exchange.java'
2--- src/main/java/com/persistit/Exchange.java 2012-11-21 16:51:18 +0000
3+++ src/main/java/com/persistit/Exchange.java 2012-11-26 15:46:22 +0000
4@@ -3237,7 +3237,8 @@
5 final int foundAt1 = search(key1, true) & P_MASK;
6 buffer = _levelCache[0]._buffer;
7 //
8- // Re-check tree generation because a structure delete could have changed
9+ // Re-check tree generation because a structure
10+ // delete could have changed
11 // search results.
12 //
13 if (_tree.getGeneration() == _cachedTreeGeneration && foundAt1 > buffer.getKeyBlockStart()
14@@ -3391,7 +3392,7 @@
15 // stitching together the pages where necessary.
16 //
17 _tree.bumpGeneration();
18-
19+
20 final long timestamp = timestamp();
21 for (int level = _cacheDepth; --level >= 0;) {
22 lc = _levelCache[level];
23
24=== modified file 'src/main/java/com/persistit/Persistit.java'
25--- src/main/java/com/persistit/Persistit.java 2012-11-21 16:32:42 +0000
26+++ src/main/java/com/persistit/Persistit.java 2012-11-26 15:46:22 +0000
27@@ -1195,10 +1195,10 @@
28 public String getProperty(final String key) {
29 return getConfiguration().getProperty(key);
30 }
31-
32+
33 @Deprecated
34 public String substituteProperties(final String text, final Properties properties) {
35- return getConfiguration().substituteProperties(text, properties);
36+ return getConfiguration().substituteProperties(text, properties);
37 }
38
39 /**
40
41=== modified file 'src/main/java/com/persistit/Value.java'
42--- src/main/java/com/persistit/Value.java 2012-11-21 15:43:27 +0000
43+++ src/main/java/com/persistit/Value.java 2012-11-26 15:46:22 +0000
44@@ -4098,6 +4098,7 @@
45 putNull();
46 return;
47 }
48+ preparePut();
49 _depth++;
50 int end = _size;
51 try {
52
53=== modified file 'src/test/java/com/persistit/BackupTaskTest.java'
54--- src/test/java/com/persistit/BackupTaskTest.java 2012-11-20 17:45:51 +0000
55+++ src/test/java/com/persistit/BackupTaskTest.java 2012-11-26 15:46:22 +0000
56@@ -15,7 +15,6 @@
57
58 package com.persistit;
59
60-import static org.junit.Assert.assertEquals;
61 import static org.junit.Assert.assertTrue;
62
63 import java.io.File;
64
65=== modified file 'src/test/java/com/persistit/Bug911849Test.java'
66--- src/test/java/com/persistit/Bug911849Test.java 2012-11-20 17:45:51 +0000
67+++ src/test/java/com/persistit/Bug911849Test.java 2012-11-26 15:46:22 +0000
68@@ -18,7 +18,6 @@
69 import static org.junit.Assert.assertEquals;
70 import static org.junit.Assert.assertTrue;
71
72-import java.util.Properties;
73 import java.util.Random;
74 import java.util.concurrent.atomic.AtomicInteger;
75
76
77=== modified file 'src/test/java/com/persistit/Bug915594Test.java'
78--- src/test/java/com/persistit/Bug915594Test.java 2012-11-20 17:45:51 +0000
79+++ src/test/java/com/persistit/Bug915594Test.java 2012-11-26 15:46:22 +0000
80@@ -17,8 +17,6 @@
81
82 import static org.junit.Assert.assertTrue;
83
84-import java.util.Properties;
85-
86 import org.junit.Test;
87
88 /**
89
90=== modified file 'src/test/java/com/persistit/Bug918909Test.java'
91--- src/test/java/com/persistit/Bug918909Test.java 2012-11-20 17:45:51 +0000
92+++ src/test/java/com/persistit/Bug918909Test.java 2012-11-26 15:46:22 +0000
93@@ -18,8 +18,6 @@
94 import static org.junit.Assert.assertFalse;
95 import static org.junit.Assert.assertTrue;
96
97-import java.util.Properties;
98-
99 import org.junit.Test;
100
101 /**
102
103=== modified file 'src/test/java/com/persistit/Bug932097Test.java'
104--- src/test/java/com/persistit/Bug932097Test.java 2012-11-20 17:45:51 +0000
105+++ src/test/java/com/persistit/Bug932097Test.java 2012-11-26 15:46:22 +0000
106@@ -15,8 +15,6 @@
107
108 package com.persistit;
109
110-import java.util.Properties;
111-
112 import org.junit.Test;
113
114 public class Bug932097Test extends PersistitUnitTestCase {
115
116=== modified file 'src/test/java/com/persistit/Bug980292Test.java'
117--- src/test/java/com/persistit/Bug980292Test.java 2012-11-20 17:45:51 +0000
118+++ src/test/java/com/persistit/Bug980292Test.java 2012-11-26 15:46:22 +0000
119@@ -17,8 +17,6 @@
120
121 import static org.junit.Assert.assertEquals;
122
123-import java.util.Properties;
124-
125 import org.junit.Test;
126
127 import com.persistit.exception.PersistitException;
128
129=== modified file 'src/test/java/com/persistit/DumpTaskTest.java'
130--- src/test/java/com/persistit/DumpTaskTest.java 2012-11-23 11:20:51 +0000
131+++ src/test/java/com/persistit/DumpTaskTest.java 2012-11-26 15:46:22 +0000
132@@ -72,7 +72,7 @@
133 final CLI cli = new CLI(_persistit, null, null);
134 final File file = File.createTempFile("DumpTaskTest", ".zip");
135 file.deleteOnExit();
136- Task task = cli.dump(file.getPath(), true, true, false);
137+ final Task task = cli.dump(file.getPath(), true, true, false);
138 task.setPersistit(_persistit);
139 task.run();
140
141
142=== modified file 'src/test/java/com/persistit/JournalManagerTest.java'
143--- src/test/java/com/persistit/JournalManagerTest.java 2012-11-20 17:45:51 +0000
144+++ src/test/java/com/persistit/JournalManagerTest.java 2012-11-26 15:46:22 +0000
145@@ -36,7 +36,6 @@
146 import java.util.LinkedList;
147 import java.util.List;
148 import java.util.Map;
149-import java.util.Properties;
150 import java.util.Random;
151 import java.util.Set;
152
153
154=== modified file 'src/test/java/com/persistit/TreeTest2.java'
155--- src/test/java/com/persistit/TreeTest2.java 2012-11-20 17:45:51 +0000
156+++ src/test/java/com/persistit/TreeTest2.java 2012-11-26 15:46:22 +0000
157@@ -19,7 +19,6 @@
158 import static org.junit.Assert.assertTrue;
159
160 import java.io.IOException;
161-import java.util.Properties;
162
163 import org.junit.Test;
164
165
166=== modified file 'src/test/java/com/persistit/VolumeTest.java'
167--- src/test/java/com/persistit/VolumeTest.java 2012-11-20 17:45:51 +0000
168+++ src/test/java/com/persistit/VolumeTest.java 2012-11-26 15:46:22 +0000
169@@ -24,7 +24,6 @@
170 import java.io.File;
171 import java.io.RandomAccessFile;
172 import java.util.List;
173-import java.util.Properties;
174
175 import org.junit.Test;
176
177
178=== modified file 'src/test/java/com/persistit/unit/ValueTest3.java'
179--- src/test/java/com/persistit/unit/ValueTest3.java 2012-08-24 13:57:19 +0000
180+++ src/test/java/com/persistit/unit/ValueTest3.java 2012-11-26 15:46:22 +0000
181@@ -40,6 +40,7 @@
182 import com.persistit.encoding.CoderManager;
183 import com.persistit.encoding.SerialValueCoder;
184 import com.persistit.encoding.ValueCoder;
185+import com.persistit.encoding.ValueRenderer;
186 import com.persistit.exception.PersistitException;
187
188 public class ValueTest3 extends PersistitUnitTestCase {
189@@ -320,9 +321,9 @@
190 private final static long serialVersionUID = 1L;
191 private final Thread _thread = Thread.currentThread(); // intentionally
192
193- // not
194- // Serializable
195+ // Externalizable -- requires a public no-arg constructor
196
197+ @SuppressWarnings("unused")
198 public EE() {
199 super();
200 }
201@@ -353,7 +354,6 @@
202
203 @Test
204 public void test1() throws PersistitException {
205- System.out.print("test1 ");
206 final S s = new S();
207 s._a = "1";
208 s._b = "2";
209@@ -361,12 +361,10 @@
210 _exchange.clear().append("test1").store();
211 final Object x = _exchange.getValue().get();
212 assertEquals("S:12", x.toString());
213- System.out.println("- done");
214 }
215
216 @Test
217 public void test2() throws PersistitException {
218- System.out.print("test2 ");
219 final SS ss = new SS("3", "4");
220 ss._a = "1";
221 ss._b = "2";
222@@ -374,12 +372,10 @@
223 _exchange.clear().append("test2").store();
224 final Object x = _exchange.getValue().get();
225 assertEquals("SS:1234", x.toString());
226- System.out.println("- done");
227 }
228
229 @Test
230 public void test3() throws PersistitException {
231- System.out.print("test3 ");
232 final E e = new E();
233 e._a = "1";
234 e._b = "2";
235@@ -387,12 +383,10 @@
236 _exchange.clear().append("test3").store();
237 final Object x = _exchange.getValue().get();
238 assertEquals("E:12", x.toString());
239- System.out.println("- done");
240 }
241
242 @Test
243 public void test4() throws PersistitException {
244- System.out.print("test4 ");
245 final EE ee = new EE("6", "7");
246 ee._a = "1";
247 ee._b = "2";
248@@ -400,34 +394,28 @@
249 _exchange.clear().append("test4").store();
250 final Object x = _exchange.getValue().get();
251 assertEquals("EE:12", x.toString());
252- System.out.println("- done");
253 }
254
255 @Test
256 public void test5() throws PersistitException {
257- System.out.print("test5 ");
258 final T t = new T("1", "2");
259 _exchange.getValue().put(t);
260 _exchange.clear().append("test5").store();
261 final Object x = _exchange.getValue().get();
262 assertEquals("T:12", x.toString());
263- System.out.println("- done");
264 }
265
266 @Test
267 public void test6() throws PersistitException {
268- System.out.print("test6 ");
269 final TT tt = new TT("1", "2");
270 _exchange.getValue().put(tt);
271 _exchange.clear().append("test6").store();
272 final Object x = _exchange.getValue().get();
273 assertEquals("TT:12", x.toString());
274- System.out.println("- done");
275 }
276
277 @Test
278 public void test7() throws PersistitException {
279- System.out.print("test7 ");
280 final CoderManager cm = _persistit.getCoderManager();
281 cm.registerValueCoder(TTT.class, new TTTValueCoder(_persistit));
282 TTTValueCoder._getCounter = 0;
283@@ -438,12 +426,10 @@
284 assertEquals("TTT:12", x.toString());
285 assertEquals(1, TTTValueCoder._getCounter);
286 cm.unregisterValueCoder(TTT.class);
287- System.out.println("- done");
288 }
289
290 @Test
291 public void test8() throws PersistitException {
292- System.out.print("test8 ");
293 final CoderManager cm = _persistit.getCoderManager();
294 cm.registerValueCoder(TTT.class, new TTTValueCoder(_persistit));
295 TTTValueCoder._getCounter = 0;
296@@ -454,12 +440,10 @@
297 assertEquals("TTT:12", x.toString());
298 assertEquals(1, TTTValueCoder._getCounter);
299 cm.unregisterValueCoder(TTT.class);
300- System.out.println("- done");
301 }
302
303 @Test
304 public void test9() throws PersistitException {
305- System.out.print("test9 ");
306 final SSS sss = new SSS("3", "4", true, 5);
307 sss._a = "1";
308 sss._b = "2";
309@@ -467,12 +451,10 @@
310 _exchange.clear().append("test9").store();
311 final Object x = _exchange.getValue().get();
312 assertEquals("SSS:1234trueW:5", x.toString());
313- System.out.println("- done");
314 }
315
316 @Test
317 public void test10() throws PersistitException {
318- System.out.print("test10 ");
319 final SSS sss = new SSS("3", "4", true, 5);
320 sss._a = "1";
321 sss._b = "2";
322@@ -480,12 +462,10 @@
323 _exchange.clear().append("test10").store();
324 final Object x = _exchange.getValue().get();
325 assertEquals("SSS:1234trueW:5", x.toString());
326- System.out.println("- done");
327 }
328
329 @Test
330 public void test11() throws PersistitException {
331- System.out.print("test11 ");
332 final SSSS ssss = new SSSS();
333 ssss._a = "Field a";
334 ssss._b = "Field b";
335@@ -494,16 +474,11 @@
336 _exchange.getValue().put(ssss);
337 _exchange.clear().append("test11").store();
338 final Object x = _exchange.getValue().get();
339- // assertEquals("SSSS:0000falsenull", x.toString());
340- System.out.print(" x=" + x + " ");
341- System.out.println();
342- System.out.println("Value: " + _exchange.getValue());
343- System.out.println("- done");
344+ assertEquals("SSSS:Field aField bSSSS-cSSSS-dtrueW:42Field gnullFinal field", x.toString());
345 }
346
347 @Test
348 public void test12() throws PersistitException {
349- System.out.print("test12 ");
350 final CoderManager cm = _persistit.getCoderManager();
351 final ValueCoder defaultCoder = cm.getValueCoder(SSSS.class);
352 _persistit.getCoderManager().registerValueCoder(SSSS.class, new SerialValueCoder(SSSS.class));
353@@ -515,34 +490,41 @@
354 _exchange.getValue().put(ssss);
355 _exchange.clear().append("test12").store();
356 final Object x = _exchange.getValue().get();
357- // assertEquals("SSSS:0000falsenull", x.toString());
358- System.out.print(" x=" + x + " ");
359- System.out.println();
360- System.out.println("Value: " + _exchange.getValue());
361+ assertEquals("SSSS:Field aField bSSSS-cSSSS-dtrueW:42Field gnullFinal field", x.toString());
362 cm.registerValueCoder(SSSS.class, defaultCoder);
363- System.out.println("- done");
364+ }
365+
366+ @Test
367+ public void testDirectPutAndGet() throws PersistitException {
368+ final Value value = _exchange.getValue();
369+ final CoderManager cm = _persistit.getCoderManager();
370+ final ValueRenderer vr = new TTTValueCoder(_persistit);
371+ cm.registerValueCoder(TTT.class, vr);
372+ TTTValueCoder._getCounter = 0;
373+ final TTT t = new TTT("1", "2");
374+ value.put(t);
375+ _exchange.clear().append("test7").store();
376+ value.clear();
377+ _exchange.fetch();
378+ final Object x = value.directGet(vr, TTT.class, null);
379+ assertEquals("TTT:12", x.toString());
380+ assertEquals(1, TTTValueCoder._getCounter);
381+ final Object y = value.directGet(vr, TTT.class, null);
382+ assertEquals("TTT:12", y.toString());
383+ final TTT z = new TTT("3", "4");
384+ value.directPut(vr, z, null);
385+ assertEquals(z.toString(), value.get().toString());
386+ assertEquals(z.toString(), value.directGet(vr, TTT.class, null).toString());
387+ value.directPut(vr, t, null);
388+ assertEquals(t.toString(), value.directGet(vr, TTT.class, null).toString());
389+ value.directPut(vr, t, null);
390+ assertEquals(t.toString(), value.directGet(vr, TTT.class, null).toString());
391+
392+ cm.unregisterValueCoder(TTT.class);
393 }
394
395 public static void main(final String[] args) throws Exception {
396 new ValueTest3().initAndRunTest();
397 }
398
399- @Override
400- public void runAllTests() throws Exception {
401- _exchange = _persistit.getExchange("persistit", "ValueTest3", true);
402-
403- test1();
404- test2();
405- test3();
406- test4();
407- test5();
408- test6();
409- test7();
410- test8();
411- test9();
412- test10();
413- test11();
414- test12();
415- }
416-
417 }

Subscribers

People subscribed via source and target branches