From 7763b8b5030d7cfbc0e1ccbfa0543e75a12c460d Mon Sep 17 00:00:00 2001 From: Erik Arvidsson Date: Mon, 27 Mar 2017 16:17:53 -0700 Subject: [PATCH] Remove OID (#3275) Fixes #3121 --- go/constants/version.go | 2 +- go/types/encoding_test.go | 2 +- go/types/type.go | 3 +- go/types/type_cache.go | 111 +----------------- go/types/type_cache_test.go | 22 ++-- go/types/type_desc.go | 27 ++++- go/types/validating_batching_sink_test.go | 2 +- .../bu62ea9gugj7k7jrqc5bjg39r2norrod | Bin 285 -> 0 bytes .../e7opaki900h5fic5mtdtlmfed3m2jjdg | Bin 372 -> 0 bytes .../eolvobg44ncok89v1c02vg5cm23t5709 | Bin 0 -> 295 bytes samples/go/hr/test-data/manifest | 2 +- .../pig8gm2iiv8aefjko2a193hnd42attpr | Bin 0 -> 373 bytes 12 files changed, 41 insertions(+), 130 deletions(-) delete mode 100644 samples/go/hr/test-data/bu62ea9gugj7k7jrqc5bjg39r2norrod delete mode 100644 samples/go/hr/test-data/e7opaki900h5fic5mtdtlmfed3m2jjdg create mode 100644 samples/go/hr/test-data/eolvobg44ncok89v1c02vg5cm23t5709 create mode 100644 samples/go/hr/test-data/pig8gm2iiv8aefjko2a193hnd42attpr diff --git a/go/constants/version.go b/go/constants/version.go index c8ecad0e20..88d1971b91 100644 --- a/go/constants/version.go +++ b/go/constants/version.go @@ -11,7 +11,7 @@ import ( ) // TODO: generate this from some central thing with go generate. -const NomsVersion = "7.2" +const NomsVersion = "7.3" const NOMS_VERSION_NEXT_ENV_NAME = "NOMS_VERSION_NEXT" const NOMS_VERSION_NEXT_ENV_VALUE = "1" diff --git a/go/types/encoding_test.go b/go/types/encoding_test.go index 71762485b1..d717073a82 100644 --- a/go/types/encoding_test.go +++ b/go/types/encoding_test.go @@ -500,7 +500,7 @@ func TestWriteListOfUnion(t *testing.T) { assertEncoding(t, // Note that the order of members in a union is determined based on a hash computation; the particular ordering of Number, Bool, String was determined empirically. This must not change unless deliberately and explicitly revving the persistent format. []interface{}{ - uint8(ListKind), uint8(UnionKind), uint32(3) /* len */, uint8(NumberKind), uint8(BoolKind), uint8(StringKind), false, + uint8(ListKind), uint8(UnionKind), uint32(3) /* len */, uint8(BoolKind), uint8(NumberKind), uint8(StringKind), false, uint32(4) /* len */, uint8(StringKind), "0", uint8(NumberKind), Number(1), uint8(StringKind), "2", uint8(BoolKind), true, }, NewList( diff --git a/go/types/type.go b/go/types/type.go index d6c1fd922d..12785195c1 100644 --- a/go/types/type.go +++ b/go/types/type.go @@ -22,7 +22,6 @@ import ( type Type struct { Desc TypeDesc h *hash.Hash - oid *hash.Hash id uint32 serialization []byte } @@ -30,7 +29,7 @@ type Type struct { const initialTypeBufferSize = 128 func newType(desc TypeDesc, id uint32) *Type { - return &Type{desc, &hash.Hash{}, nil, id, nil} + return &Type{desc, &hash.Hash{}, id, nil} } func ensureTypeSerialization(t *Type) { diff --git a/go/types/type_cache.go b/go/types/type_cache.go index 1f3e7ac7c2..7a2450c3ca 100644 --- a/go/types/type_cache.go +++ b/go/types/type_cache.go @@ -236,9 +236,6 @@ func checkStructType(t *Type, checkKind checkKindType) { return } - walkType(t, nil, generateOIDs) - walkType(t, nil, checkForUnrolledCycles) - switch checkKind { case checkKindNormalize: walkType(t, nil, sortUnions) @@ -249,20 +246,6 @@ func checkStructType(t *Type, checkKind checkKindType) { } } -func generateOIDs(t *Type, _ []*Type) { - generateOID(t, false) -} - -func checkForUnrolledCycles(t *Type, parentStructTypes []*Type) { - if t.Kind() == StructKind { - for _, ttt := range parentStructTypes { - if *t.oid == *ttt.oid { - panic("unrolled cycle types are not supported; ahl owes you a beer") - } - } - } -} - func sortUnions(t *Type, _ []*Type) { if t.Kind() == UnionKind { sort.Sort(t.Desc.(CompoundDesc).ElemTypes) @@ -277,7 +260,7 @@ func validateTypes(t *Type, _ []*Type) { panic("Invalid union type") } for i := 1; i < len(elemTypes); i++ { - if !elemTypes[i-1].oid.Less(*elemTypes[i].oid) { + if !unionLess(elemTypes[i-1], elemTypes[i]) { panic("Invalid union order") } } @@ -309,95 +292,6 @@ func walkType(t *Type, parentStructTypes []*Type, cb func(*Type, []*Type)) { } } -func generateOID(t *Type, allowUnresolvedCycles bool) { - if t.oid == nil { - buf := newBinaryNomsWriter() - encodeForOID(t, buf, allowUnresolvedCycles, t, nil) - oid := hash.Of(buf.data()) - t.oid = &oid - } -} - -func encodeForOID(t *Type, buf nomsWriter, allowUnresolvedCycles bool, root *Type, parentStructTypes []*Type) { - // Most types are encoded in a straightforward fashion - switch desc := t.Desc.(type) { - case CycleDesc: - if allowUnresolvedCycles { - buf.writeUint8(uint8(desc.Kind())) - buf.writeUint32(uint32(desc)) - } else { - panic("found an unexpected unresolved cycle") - } - case PrimitiveDesc: - buf.writeUint8(uint8(desc.Kind())) - case CompoundDesc: - switch k := desc.Kind(); k { - case ListKind, MapKind, RefKind, SetKind: - buf.writeUint8(uint8(k)) - buf.writeUint32(uint32(len(desc.ElemTypes))) - for _, tt := range desc.ElemTypes { - encodeForOID(tt, buf, allowUnresolvedCycles, root, parentStructTypes) - } - case UnionKind: - buf.writeUint8(uint8(k)) - if t == root { - // If this is where we started we don't need to keep going - return - } - - buf.writeUint32(uint32(len(desc.ElemTypes))) - - // This is the only subtle case: encode each subordinate type, generate the hash, remove duplicates, and xor the results together to form an order independent encoding. - mbuf := newBinaryNomsWriter() - oids := make(map[hash.Hash]struct{}) - for _, elemType := range desc.ElemTypes { - h := elemType.oid - if h == nil { - - mbuf.reset() - encodeForOID(elemType, mbuf, allowUnresolvedCycles, root, parentStructTypes) - h2 := hash.Of(mbuf.data()) - if _, found := indexOfType(elemType, parentStructTypes); !found { - elemType.oid = &h2 - } - oids[h2] = struct{}{} - } else { - oids[*h] = struct{}{} - if !allowUnresolvedCycles { - checkForUnresolvedCycles(elemType, root, parentStructTypes) - } - } - } - - data := make([]byte, hash.ByteLen) - for o := range oids { - for i := 0; i < len(data); i++ { - data[i] ^= o[i] - } - } - buf.writeBytes(data) - default: - panic("unknown compound type") - } - case StructDesc: - idx, found := indexOfType(t, parentStructTypes) - if found { - buf.writeUint8(uint8(CycleKind)) - buf.writeUint32(uint32(len(parentStructTypes)) - 1 - idx) - return - } - - buf.writeUint8(uint8(StructKind)) - buf.writeString(desc.Name) - - parentStructTypes = append(parentStructTypes, t) - for _, field := range desc.fields { - buf.writeString(field.name) - encodeForOID(field.t, buf, allowUnresolvedCycles, root, parentStructTypes) - } - } -} - func checkForUnresolvedCycles(t, root *Type, parentStructTypes []*Type) { desc := t.Desc @@ -433,9 +327,6 @@ func (tc *TypeCache) makeUnionType(elemTypes ...*Type) *Type { if len(ts) == 1 { return ts[0] } - for _, tt := range ts { - generateOID(tt, true) - } // We sort the contituent types to dedup equivalent types in memory; we may need to sort again after cycles are resolved for final encoding. sort.Sort(ts) return tc.getCompoundType(UnionKind, ts...) diff --git a/go/types/type_cache_test.go b/go/types/type_cache_test.go index 423dfc9350..3be4b1e373 100644 --- a/go/types/type_cache_test.go +++ b/go/types/type_cache_test.go @@ -197,9 +197,9 @@ func TestTypeCacheCyclicUnions(t *testing.T) { []*Type{ut}, ) - assert.True(ut.Desc.(CompoundDesc).ElemTypes[0].Kind() == CycleKind) - // That the Struct / Cycle landed in index 1 was found empirically. - assert.True(st == st.Desc.(StructDesc).fields[0].t.Desc.(CompoundDesc).ElemTypes[1]) + assert.True(ut.Desc.(CompoundDesc).ElemTypes[6].Kind() == CycleKind) + // That the Struct / Cycle landed in index 5 was found empirically. + assert.Equal(st, st.Desc.(StructDesc).fields[0].t.Desc.(CompoundDesc).ElemTypes[5]) // ut contains an explicit Cycle type; noms must not surrepticiously change existing types so we can be sure that the Union within st is different in that the cycle has been resolved. assert.False(ut == st.Desc.(StructDesc).fields[0].t) @@ -209,22 +209,22 @@ func TestTypeCacheCyclicUnions(t *testing.T) { []string{"foo"}, []*Type{ut2}, ) - assert.True(ut2.Desc.(CompoundDesc).ElemTypes[0].Kind() == CycleKind) - assert.True(st2 == st2.Desc.(StructDesc).fields[0].t.Desc.(CompoundDesc).ElemTypes[1]) + assert.True(ut2.Desc.(CompoundDesc).ElemTypes[6].Kind() == CycleKind) + assert.True(st2 == st2.Desc.(StructDesc).fields[0].t.Desc.(CompoundDesc).ElemTypes[5]) assert.False(ut2 == st2.Desc.(StructDesc).fields[0].t) assert.True(ut == ut2) assert.True(st == st2) } -func TestInvalidCyclesAndUnions(t *testing.T) { +func TestNonNormalizedCycles(t *testing.T) { assert := assert.New(t) - assert.Panics(func() { - MakeStructType("A", - []string{"a"}, - []*Type{MakeStructType("A", []string{"a"}, []*Type{MakeCycleType(1)})}) - }) + t1 := MakeStructType("A", + []string{"a"}, + []*Type{MakeStructType("A", []string{"a"}, []*Type{MakeCycleType(1)})}) + t2 := t1.Desc.(StructDesc).fields[0].t + assert.True(t1.Equals(t2)) } func TestMakeStructTypeFromFields(t *testing.T) { diff --git a/go/types/type_desc.go b/go/types/type_desc.go index 1a1bb83b3f..79ea337da4 100644 --- a/go/types/type_desc.go +++ b/go/types/type_desc.go @@ -137,6 +137,27 @@ func (c CycleDesc) HasUnresolvedCycle(visited []*Type) bool { type typeSlice []*Type -func (ts typeSlice) Len() int { return len(ts) } -func (ts typeSlice) Less(i, j int) bool { return ts[i].oid.Less(*ts[j].oid) } -func (ts typeSlice) Swap(i, j int) { ts[i], ts[j] = ts[j], ts[i] } +func (ts typeSlice) Len() int { return len(ts) } + +func (ts typeSlice) Less(i, j int) bool { + return unionLess(ts[i], ts[j]) +} + +func (ts typeSlice) Swap(i, j int) { ts[i], ts[j] = ts[j], ts[i] } + +func unionLess(ti, tj *Type) bool { + if ti == tj { + return false + } + + ki, kj := ti.Kind(), tj.Kind() + if ki == kj { + if ki == StructKind { + // Due to type simplification, the only thing that matters is the name of the struct. + return ti.Desc.(StructDesc).Name < tj.Desc.(StructDesc).Name + } + + return false + } + return ki < kj +} diff --git a/go/types/validating_batching_sink_test.go b/go/types/validating_batching_sink_test.go index 5e5314c03d..a580c91526 100644 --- a/go/types/validating_batching_sink_test.go +++ b/go/types/validating_batching_sink_test.go @@ -71,7 +71,7 @@ func assertPanicsOnInvalidChunk(t *testing.T, data []interface{}) { func TestValidatingBatchingSinkDecodeInvalidUnion(t *testing.T) { data := []interface{}{ uint8(TypeKind), - uint8(UnionKind), uint32(2) /* len */, uint8(BoolKind), uint8(NumberKind), + uint8(UnionKind), uint32(2) /* len */, uint8(NumberKind), uint8(BoolKind), } assertPanicsOnInvalidChunk(t, data) } diff --git a/samples/go/hr/test-data/bu62ea9gugj7k7jrqc5bjg39r2norrod b/samples/go/hr/test-data/bu62ea9gugj7k7jrqc5bjg39r2norrod deleted file mode 100644 index c7fe4c3a8ba2807559cd4dc20711b66e0c9951b8..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 285 zcmeyv7{STFz`*95pPQRm0%S1*F-vZ0Ng|`TAfpC*L1IyAUP&`2aef}JZ3I(h3L{8QUSe)46Oh4Ll39|I%EYM2%E-dO!^+OcxXPY^QD1_Q zQHlX*)wBWz4j_xs)PgTKF}E}|N5LsSKP5LYk5O2MJ1js)!MQZ2q_ik?YJQoR-WE0{ zb{3$S42&R?nKFvlSzer7y;*+d!}|5v3hcqE>of}_W+#GF00HAh+nZmXiWju^^z1ML u@nLGgvJ4D!fwT#-zWq1M3`7t73avUgT@5G)vI7Vhwf=9taY!ZkT>t>mgh`2aef}JEe}&>3L~S7MP6cVDicsIYe{BFPAU_lCMzQg2RkD-1KYnshNeOW z##{!*e>n`Zau|GGoL#+He&)ma_1OyS!K&*t3nXSIGB)r4H8ZYKVPrJmVPfQCP*v;; zW#j-d7)?!h9TSW4^Aw!&^HXvY^B9FPB%CucQ_~b&%M$a_Q*$zla~1s4(lV1%ix{O= z#W65QvBpX81t;c~rsn9yGw5)K1?VU^m*$j|7NweRX*j31g^h`w1!x@uBgn-}8Aa?Y zEgG}etU7e#fz%WK-Xo8n&CeF7`U+A31Wb81^^;-pAc0wrD-BOIZ72gW8G-l<5St+D hTWx38@b5`j)wa`obAfUoJ3v(K|E)I;sU*J(006;NY^4AI diff --git a/samples/go/hr/test-data/eolvobg44ncok89v1c02vg5cm23t5709 b/samples/go/hr/test-data/eolvobg44ncok89v1c02vg5cm23t5709 new file mode 100644 index 0000000000000000000000000000000000000000..847bb89a85ad5ff9a99f23afe160614ed165db47 GIT binary patch literal 295 zcmeBRir{2mU|@63&&|y&0kW8Zm?by0B#}{EkWquZAh9SlucVlRotsx!hP5m)r!8)c_i#&@*UfshOFezcz09=5wVOdIfPgV( zR(Bpu9whKyak23YO|cM=91t%DViRP2FPpMboht8jT(;U44wM7g0R)U@|F_;aq>}tD F001_{O^5&h literal 0 HcmV?d00001 diff --git a/samples/go/hr/test-data/manifest b/samples/go/hr/test-data/manifest index 21cfe76c45..a218c06169 100644 --- a/samples/go/hr/test-data/manifest +++ b/samples/go/hr/test-data/manifest @@ -1 +1 @@ -2:7.2:jbhniceagao7dapu7q0ftp2mfarcm3kt:bu62ea9gugj7k7jrqc5bjg39r2norrod:2:e7opaki900h5fic5mtdtlmfed3m2jjdg:2 \ No newline at end of file +2:7.3:rop7vcma6nt5sg0m6uk81kdsaiqmuinl:pig8gm2iiv8aefjko2a193hnd42attpr:2:eolvobg44ncok89v1c02vg5cm23t5709:2 \ No newline at end of file diff --git a/samples/go/hr/test-data/pig8gm2iiv8aefjko2a193hnd42attpr b/samples/go/hr/test-data/pig8gm2iiv8aefjko2a193hnd42attpr new file mode 100644 index 0000000000000000000000000000000000000000..d1ee83cae0f4f5ef0825ff0efc892d2c48a4df23 GIT binary patch literal 373 zcmdnb62Zy9z`*95pPQRm0%S1*F-vZ0Ng|`TAfpC*L1IyAUP&`2aef}JEe}&>3L~S7MP6cVDicsIYe{BFPAU_lCMzQg2RkD-1KYnshNeOW z##{!*e>n`Zau|HxOHKE1OPSZrdbLYEeagMew8gcX85?+jni*HAFftklFfsBma4Jsf z3uWX0vKUQGcpVdq^79m&^7B)26Z05_G9;WcGE>tOT+0&k(o=IXi*ptH)6z1NQ;Qg- zR>d(eNU_FA@C7I4mZs+D#WUz|hXv>;IG5&>loq9CAJCYuw}p*~odswg10%@EOc_P& zEUc^E^_xza{sLOWzzD=&f!G9D gpM#kBiiV4OLbm36f#g7TfT-O6TW=gvNq!dq079l|00000 literal 0 HcmV?d00001