Skip to content

Commit

Permalink
Small InputStream cleanup (#3211)
Browse files Browse the repository at this point in the history
  • Loading branch information
bernardnormier authored Nov 28, 2024
1 parent ade64da commit b209ddf
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 116 deletions.
6 changes: 1 addition & 5 deletions cpp/include/Ice/InputStream.h
Original file line number Diff line number Diff line change
Expand Up @@ -798,8 +798,6 @@ namespace Ice

std::string resolveCompactId(int) const;

void postUnmarshal(const ValuePtr&) const;

class Encaps;
enum SliceType
{
Expand Down Expand Up @@ -1071,10 +1069,8 @@ namespace Ice
int _startSeq;
int _minSeqSize;

// These values are retrieved from instance during construction and cached.
// Retrieved from instance during construction and cached. Never null.
const ValueFactoryManagerPtr _valueFactoryManager;
const LoggerPtr _logger;
const std::function<std::string(int)> _compactIdResolver;

std::vector<std::function<void()>> _deleters;
};
Expand Down
78 changes: 25 additions & 53 deletions cpp/src/Ice/InputStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1061,10 +1061,9 @@ Ice::InputStream::InputStream(Instance* instance, EncodingVersion encoding, Buff
_closure(nullptr),
_startSeq(-1),
_minSeqSize(0),
_valueFactoryManager(instance->initializationData().valueFactoryManager),
_logger(instance->initializationData().logger),
_compactIdResolver(instance->initializationData().compactIdResolver)
_valueFactoryManager(instance->initializationData().valueFactoryManager)
{
assert(_valueFactoryManager);
}

ReferencePtr
Expand Down Expand Up @@ -1263,12 +1262,14 @@ Ice::InputStream::throwUnmarshalOutOfBoundsException(const char* file, int line)
string
Ice::InputStream::resolveCompactId(int id) const
{
string type;
if (_compactIdResolver)
string typeId;
const std::function<std::string(int)>& compactIdResolver = _instance->initializationData().compactIdResolver;

if (compactIdResolver)
{
try
{
type = _compactIdResolver(id);
typeId = compactIdResolver(id);
}
catch (const LocalException&)
{
Expand All @@ -1294,40 +1295,25 @@ Ice::InputStream::resolveCompactId(int id) const
}
}

return type;
}

void
Ice::InputStream::postUnmarshal(const shared_ptr<Value>& v) const
{
try
if (typeId.empty())
{
v->ice_postUnmarshal();
}
catch (const std::exception& ex)
{
if (_logger)
{
Warning out(_logger);
out << "std::exception raised by ice_postUnmarshal:\n" << ex;
}
}
catch (...)
{
if (_logger)
{
Warning out(_logger);
out << "unknown exception raised by ice_postUnmarshal";
}
typeId = IceInternal::factoryTable->getTypeId(id);
}

return typeId;
}

void
Ice::InputStream::traceSkipSlice(string_view typeId, SliceType sliceType) const
{
if (_instance->traceLevels()->slicing > 0 && _logger)
assert(_instance->initializationData().logger); // not null once the communicator is initialized
if (_instance->traceLevels()->slicing > 0)
{
traceSlicing(sliceType == ExceptionSlice ? "exception" : "object", typeId, "Slicing", _logger);
traceSlicing(
sliceType == ExceptionSlice ? "exception" : "object",
typeId,
"Slicing",
_instance->initializationData().logger);
}
}

Expand Down Expand Up @@ -1389,20 +1375,14 @@ Ice::InputStream::EncapsDecoder::newInstance(string_view typeId)
shared_ptr<Value> v;

// Try to find a factory registered for the specific type.
function<shared_ptr<Value>(string_view)> userFactory;
if (_valueFactoryManager)
function<shared_ptr<Value>(string_view)> userFactory = _valueFactoryManager->find(typeId);
if (userFactory)
{
userFactory = _valueFactoryManager->find(typeId);
if (userFactory)
{
v = userFactory(typeId);
}
v = userFactory(typeId);
}

//
// If that fails, invoke the default factory if one has been registered.
//
if (!v && _valueFactoryManager)
if (!v)
{
userFactory = _valueFactoryManager->find("");
if (userFactory)
Expand Down Expand Up @@ -1517,23 +1497,21 @@ Ice::InputStream::EncapsDecoder::unmarshal(int32_t index, const ValuePtr& v)

if (_valueList.empty() && _patchMap.empty())
{
_stream->postUnmarshal(v);
v->ice_postUnmarshal();
}
else
{
_valueList.push_back(v);

if (_patchMap.empty())
{
//
// Iterate over the value list and invoke ice_postUnmarshal on
// each value. We must do this after all values have been
// unmarshaled in order to ensure that any value data members
// have been properly patched.
//
for (ValueList::iterator p = _valueList.begin(); p != _valueList.end(); ++p)
for (auto& value : _valueList)
{
_stream->postUnmarshal(*p);
value->ice_postUnmarshal();
}
_valueList.clear();
}
Expand Down Expand Up @@ -2207,14 +2185,8 @@ Ice::InputStream::EncapsDecoder11::readInstance(int32_t index, PatchFunc patchFu
{
if (_current->compactId >= 0)
{
//
// Translate a compact (numeric) type ID into a string type ID.
//
_current->typeId = _stream->resolveCompactId(_current->compactId);
if (_current->typeId.empty())
{
_current->typeId = IceInternal::factoryTable->getTypeId(_current->compactId);
}
}

if (!_current->typeId.empty())
Expand Down
20 changes: 2 additions & 18 deletions csharp/src/Ice/InputStream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2553,15 +2553,7 @@ protected void unmarshal(int index, Value v)

if ((_patchMap == null || _patchMap.Count == 0) && _valueList == null)
{
try
{
v.ice_postUnmarshal();
}
catch (System.Exception ex)
{
string s = "exception raised by ice_postUnmarshal:\n" + ex;
_stream.instance().initializationData().logger!.warning(s);
}
v.ice_postUnmarshal();
}
else
{
Expand All @@ -2581,15 +2573,7 @@ protected void unmarshal(int index, Value v)
//
foreach (var p in _valueList)
{
try
{
p.ice_postUnmarshal();
}
catch (System.Exception ex)
{
string s = "exception raised by ice_postUnmarshal:\n" + ex;
_stream.instance().initializationData().logger!.warning(s);
}
p.ice_postUnmarshal();
}
_valueList.Clear();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,9 @@ private InputStream(Instance instance, EncodingVersion encoding, Buffer buf) {
// Everything below is cached from instance.
_classGraphDepthMax = _instance.classGraphDepthMax();
_valueFactoryManager = _instance.initializationData().valueFactoryManager;
_logger = _instance.initializationData().logger;
_classResolver = _instance;

assert (_valueFactoryManager != null);
assert (_logger != null);
}

/**
Expand Down Expand Up @@ -1744,12 +1742,7 @@ protected void unmarshal(int index, Value v) {
}

if ((_patchMap == null || _patchMap.isEmpty()) && _valueList == null) {
try {
v.ice_postUnmarshal();
} catch (Exception ex) {
String s = "exception raised by ice_postUnmarshal:\n" + Ex.toString(ex);
_stream.instance().initializationData().logger.warning(s);
}
v.ice_postUnmarshal();
} else {
if (_valueList == null) // Lazy initialization
{
Expand All @@ -1758,19 +1751,12 @@ protected void unmarshal(int index, Value v) {
_valueList.add(v);

if (_patchMap == null || _patchMap.isEmpty()) {
//
// Iterate over the instance list and invoke ice_postUnmarshal on
// each instance. We must do this after all instances have been
// unmarshaled in order to ensure that any instance data members
// have been properly patched.
//
for (Value p : _valueList) {
try {
p.ice_postUnmarshal();
} catch (Exception ex) {
String s = "exception raised by ice_postUnmarshal:\n" + Ex.toString(ex);
_stream.instance().initializationData().logger.warning(s);
}
p.ice_postUnmarshal();
}
_valueList.clear();
}
Expand Down Expand Up @@ -2651,7 +2637,7 @@ private void traceSkipSlice(String typeId, SliceType sliceType) {
sliceType == SliceType.ExceptionSlice ? "exception" : "object",
typeId,
"Slicing",
_logger);
_instance.initializationData().logger);
}
}

Expand Down Expand Up @@ -2698,7 +2684,6 @@ public static interface Unmarshaler {
private int _minSeqSize;

private final ValueFactoryManager _valueFactoryManager;
private final Logger _logger;
private final java.util.function.Function<String, Class<?>> _classResolver;

private static final String END_OF_BUFFER_MESSAGE =
Expand Down
6 changes: 2 additions & 4 deletions js/src/Ice/InputStream.js
Original file line number Diff line number Diff line change
Expand Up @@ -963,7 +963,6 @@ export class InputStream {
this._sizePos = -1;

this._valueFactoryManager = this._instance.initializationData().valueFactoryManager;
this._logger = this._instance.initializationData().logger;
this._classGraphDepthMax = this._instance.classGraphDepthMax();
}

Expand All @@ -985,7 +984,6 @@ export class InputStream {
// These are cached values derived from instance.
DEV: console.assert(this._classGraphDepthMax === other._classGraphDepthMax);
DEV: console.assert(this._valueFactoryManager === other._valueFactoryManager);
DEV: console.assert(this._logger === other._logger);

[other._buf, this._buf] = [this._buf, other._buf];
[other._encoding, this._encoding] = [this._encoding, other._encoding];
Expand Down Expand Up @@ -1622,12 +1620,12 @@ export class InputStream {
}

traceSkipSlice(typeId, sliceType) {
if (this._instance.traceLevels().slicing > 0 && this._logger !== null) {
if (this._instance.traceLevels().slicing > 0) {
traceSlicing(
sliceType === SliceType.ExceptionSlice ? "exception" : "object",
typeId,
"Slicing",
this._logger,
this._instance.initializationData().logger,
);
}
}
Expand Down
21 changes: 3 additions & 18 deletions matlab/lib/+IceInternal/EncapsDecoder.m
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,7 @@ function finish(obj)
%
for i = 1:length(obj.delayedPostUnmarshal)
v = obj.delayedPostUnmarshal{i};
try
v.ice_postUnmarshal();
catch ex
msg = sprintf('exception raised by ice_postUnmarshal:\n%s', getReport(ex, 'extended'));
obj.is.getCommunicator().getLogger().warning(msg);
end
v.ice_postUnmarshal();
end

obj.delayedPostUnmarshal = {};
Expand Down Expand Up @@ -195,12 +190,7 @@ function unmarshal(obj, index, v)
if v.iceDelayPostUnmarshal()
obj.delayedPostUnmarshal{end + 1} = v; % See finish()
else
try
v.ice_postUnmarshal();
catch ex
msg = sprintf('exception raised by ice_postUnmarshal:\n%s', getReport(ex, 'extended'));
obj.is.getCommunicator().getLogger().warning(msg);
end
v.ice_postUnmarshal();
end
else
obj.valueList{end + 1} = v;
Expand All @@ -217,12 +207,7 @@ function unmarshal(obj, index, v)
if p.iceDelayPostUnmarshal()
obj.delayedPostUnmarshal{end + 1} = p; % See finish()
else
try
p.ice_postUnmarshal();
catch ex
msg = sprintf('exception raised by ice_postUnmarshal:\n%s', getReport(ex, 'extended'));
obj.is.getCommunicator().getLogger().warning(msg);
end
p.ice_postUnmarshal();
end
end
obj.valueList = {};
Expand Down

0 comments on commit b209ddf

Please sign in to comment.