Skip to content

Commit d9b432b

Browse files
cosminbascacosminbasca
committed
[GR-10976] pypy style computed ids for builtin immutable types
PullRequest: graalpython/133
2 parents a3eb04b + b9cd784 commit d9b432b

File tree

6 files changed

+63
-103
lines changed

6 files changed

+63
-103
lines changed

‎graalpython/com.oracle.graal.python.test/src/tests/test_string.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22
# Copyright (C) 1996-2017 Python Software Foundation
33
#
44
# Licensed under the PYTHON SOFTWARE FOUNDATION LICENSE VERSION 2
5-
import sys
65
import unittest
76

7+
import sys
8+
9+
810
def test_find():
911
assert "teststring".find("test") == 0
1012
assert "teststring".find("string") == 4
@@ -681,3 +683,12 @@ def test_isprintable(self):
681683

682684
self.assertTrue('\U0001F46F'.isprintable())
683685
# self.assertFalse('\U000E0020'.isprintable())
686+
687+
688+
def test_same_id():
689+
empty_ids = set([id(str()) for i in range(100)])
690+
assert len(empty_ids) == 1
691+
empty_ids = set([id('') for i in range(100)])
692+
assert len(empty_ids) == 1
693+
empty_ids = set([id(u'') for i in range(100)])
694+
assert len(empty_ids) == 1

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/PythonImmutableBuiltinType.java

Lines changed: 0 additions & 47 deletions
This file was deleted.

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/BuiltinFunctions.java

Lines changed: 43 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@
7575
import com.oracle.graal.python.builtins.Builtin;
7676
import com.oracle.graal.python.builtins.CoreFunctions;
7777
import com.oracle.graal.python.builtins.PythonBuiltins;
78-
import com.oracle.graal.python.builtins.PythonImmutableBuiltinType;
7978
import com.oracle.graal.python.builtins.objects.PNone;
8079
import com.oracle.graal.python.builtins.objects.PNotImplemented;
8180
import com.oracle.graal.python.builtins.objects.bytes.PBytes;
@@ -634,6 +633,19 @@ public abstract static class IdNode extends PythonBuiltinNode {
634633
* </pre>
635634
*/
636635
private static long KNOWN_OBJECTS_COUNT = 4L;
636+
// borrowed logic from pypy
637+
// -1 - (-maxunicode-1): unichar
638+
// 0 - 255: char
639+
// 256: empty string
640+
// 257: empty unicode
641+
// 258: empty tuple
642+
// 259: empty frozenset
643+
private static long BASE_EMPTY_BYTES = 256;
644+
private static long BASE_EMPTY_UNICODE = 257;
645+
private static long BASE_EMPTY_TUPLE = 258;
646+
private static long BASE_EMPTY_FROZENSET = 259;
647+
private static long IDTAG_SPECIAL = 11;
648+
private static int IDTAG_SHIFT = 4;
637649

638650
/**
639651
* The next available global id. We reserve space for all integers to be their own id +
@@ -665,17 +677,6 @@ long doId(int integer) {
665677
return integer + KNOWN_OBJECTS_COUNT;
666678
}
667679

668-
/**
669-
* TODO: {@link #doId(String)} and {@link #doId(double)} are not quite right, because the
670-
* hashCode will certainly collide with integer hashes. It should be good for comparisons
671-
* between String and String id, though, it'll just look as if we interned all strings from
672-
* the Python code's perspective.
673-
*/
674-
@Specialization
675-
int doId(String value) {
676-
return value.hashCode();
677-
}
678-
679680
@Specialization
680681
Object doId(PInt value) {
681682
try {
@@ -691,26 +692,49 @@ int doId(double value) {
691692
}
692693

693694
@Specialization(guards = "isEmpty(value)")
694-
Object doEmptyTuple(PTuple value) {
695-
return getId(value, PythonImmutableBuiltinType.PTuple);
695+
Object doEmptyString(@SuppressWarnings("unused") String value) {
696+
return (BASE_EMPTY_UNICODE << IDTAG_SHIFT) | IDTAG_SPECIAL;
697+
}
698+
699+
@Specialization(guards = "isEmpty(value)")
700+
Object doEmptyString(@SuppressWarnings("unused") PString value) {
701+
return (BASE_EMPTY_UNICODE << IDTAG_SHIFT) | IDTAG_SPECIAL;
696702
}
697703

698704
@Specialization(guards = "isEmpty(value)")
699-
Object doEmptyBytes(PBytes value) {
700-
return getId(value, PythonImmutableBuiltinType.PBytes);
705+
Object doEmptyTuple(@SuppressWarnings("unused") PTuple value) {
706+
return (BASE_EMPTY_TUPLE << IDTAG_SHIFT) | IDTAG_SPECIAL;
701707
}
702708

703709
@Specialization(guards = "isEmpty(value)")
704-
Object doEmptyFrozenSet(PFrozenSet value) {
705-
return getId(value, PythonImmutableBuiltinType.PFrozenSet);
710+
Object doEmptyBytes(@SuppressWarnings("unused") PBytes value) {
711+
return (BASE_EMPTY_BYTES << IDTAG_SHIFT) | IDTAG_SPECIAL;
712+
}
713+
714+
@Specialization(guards = "isEmpty(value)")
715+
Object doEmptyFrozenSet(@SuppressWarnings("unused") PFrozenSet value) {
716+
return (BASE_EMPTY_FROZENSET << IDTAG_SHIFT) | IDTAG_SPECIAL;
706717
}
707718

708719
protected boolean isEmptyImmutableBuiltin(Object object) {
709720
return (object instanceof PTuple && PGuards.isEmpty((PTuple) object)) ||
721+
(object instanceof String && PGuards.isEmpty((String) object)) ||
722+
(object instanceof PString && PGuards.isEmpty((PString) object)) ||
710723
(object instanceof PBytes && PGuards.isEmpty((PBytes) object)) ||
711724
(object instanceof PFrozenSet && PGuards.isEmpty((PFrozenSet) object));
712725
}
713726

727+
/**
728+
* TODO: {@link #doId(String)} and {@link #doId(double)} are not quite right, because the
729+
* hashCode will certainly collide with integer hashes. It should be good for comparisons
730+
* between String and String id, though, it'll just look as if we interned all strings from
731+
* the Python code's perspective.
732+
*/
733+
@Specialization(guards = "!isEmpty(value)")
734+
int doId(String value) {
735+
return value.hashCode();
736+
}
737+
714738
@Specialization(guards = {"!isPInt(obj)", "!isPString(obj)", "!isPFloat(obj)", "!isEmptyImmutableBuiltin(obj)"})
715739
Object doId(PythonObject obj) {
716740
return getId(obj);
@@ -722,22 +746,14 @@ Object doId(Object obj) {
722746
}
723747

724748
private Object getId(PythonObject obj) {
725-
return getId(obj, null);
726-
}
727-
728-
private Object getId(PythonObject obj, PythonImmutableBuiltinType immutableType) {
729749
if (readId == null) {
730750
CompilerDirectives.transferToInterpreterAndInvalidate();
731751
readId = insert(ReadAttributeFromObjectNode.create());
732752
writeId = insert(WriteAttributeToObjectNode.create());
733753
}
734754
Object id = readId.execute(obj, ID_KEY);
735755
if (id == NO_VALUE) {
736-
if (immutableType != null) {
737-
id = getContext().getEmptyImmutableObjectGlobalId(immutableType);
738-
} else {
739-
id = getContext().getNextGlobalId();
740-
}
756+
id = getContext().getNextGlobalId();
741757
writeId.execute(obj, ID_KEY, id);
742758
}
743759
return id;

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/PGuards.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,14 @@ public static boolean isEmpty(PBytes bytes) {
9999
return bytes.len() == 0;
100100
}
101101

102+
public static boolean isEmpty(String string) {
103+
return string.length() == 0;
104+
}
105+
106+
public static boolean isEmpty(PString string) {
107+
return string.len() == 0;
108+
}
109+
102110
public static boolean isNone(Object value) {
103111
return value == PNone.NONE;
104112
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/runtime/PythonContext.java

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@
3737
import org.graalvm.options.OptionValues;
3838

3939
import com.oracle.graal.python.PythonLanguage;
40-
import com.oracle.graal.python.builtins.PythonImmutableBuiltinType;
4140
import com.oracle.graal.python.builtins.objects.common.HashingStorage;
4241
import com.oracle.graal.python.builtins.objects.dict.PDict;
4342
import com.oracle.graal.python.builtins.objects.module.PythonModule;
@@ -59,8 +58,6 @@ public class PythonContext {
5958
private final HashMap<Object, CallTarget> atExitHooks = new HashMap<>();
6059
private final AtomicLong globalId = new AtomicLong(Integer.MAX_VALUE * 2L + 4L);
6160

62-
private final long[] emptyImmutableObjectsIdCache = new long[PythonImmutableBuiltinType.values().length];
63-
6461
@CompilationFinal private TruffleLanguage.Env env;
6562

6663
private PException currentException;
@@ -94,18 +91,6 @@ public PythonContext(PythonLanguage language, TruffleLanguage.Env env, PythonCor
9491
}
9592
}
9693

97-
public long getEmptyImmutableObjectGlobalId(PythonImmutableBuiltinType immutableType) {
98-
int idx = immutableType.ordinal();
99-
if (emptyImmutableObjectsIdCache[idx] == 0) {
100-
synchronized (emptyImmutableObjectsIdCache) {
101-
if (emptyImmutableObjectsIdCache[idx] == 0) {
102-
emptyImmutableObjectsIdCache[idx] = getNextGlobalId();
103-
}
104-
}
105-
}
106-
return emptyImmutableObjectsIdCache[idx];
107-
}
108-
10994
@TruffleBoundary(allowInlining = true)
11095
public long getNextGlobalId() {
11196
return globalId.incrementAndGet();

‎graalpython/lib-graalpython/set.py

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -132,16 +132,3 @@ def frozenset_hash(self):
132132
frozenset.__repr__ = frozenset_repr
133133
frozenset.copy = frozenset_copy
134134
frozenset.__hash__ = frozenset_hash
135-
136-
del update
137-
del difference
138-
del difference_update
139-
del intersection
140-
del set_repr
141-
del set_copy
142-
143-
del frozenset_difference
144-
del frozenset_intersection
145-
del frozenset_repr
146-
del frozenset_copy
147-
del frozenset_hash

0 commit comments

Comments
 (0)