Skip to content

Mapsicle.Fluent's in-place Map(source, destination) is 17x slower than the core and allocates 776 bytes per call #8

Description

@arnelirobles

IMapper.Map<TSource, TDest>(TSource source, TDest destination) does not compile or cache anything.
It reflects over both types and calls PropertyInfo.SetValue for every property, on every call.

Measured, 100,000 calls, five-property User to UserDto, warm, Release, .NET 8:

Call Time Allocation
Mapsicle.Fluent Map<TSource,TDest>(source, destination) 190 ms 776 B/call
core Mapsicle Map(destination) 11 ms 0 B/call
Mapsicle.Fluent Map<TDest>(source) 83 ms 112 B/call
core Mapsicle MapTo<T>(source) 10 ms 48 B/call

Current behaviour. src/Mapsicle.Fluent/FluentMapper.cs:425:

public TDest Map<TSource, TDest>(TSource source, TDest destination)
{
    if (source is null || destination is null) return destination;

    var typeMap = _config.GetTypeMap(typeof(TSource), typeof(TDest));
    var sourceProps = typeof(TSource).GetProperties(BindingFlags.Public | BindingFlags.Instance);   // :430
    var destProps = typeof(TDest).GetProperties(BindingFlags.Public | BindingFlags.Instance);       // :431
    ...
        var sourceProp = sourceProps.FirstOrDefault(p =>
            p.Name.Equals(destProp.Name, StringComparison.OrdinalIgnoreCase) && p.CanRead);         // :454

        if (sourceProp != null && destProp.PropertyType.IsAssignableFrom(sourceProp.PropertyType))
        {
            destProp.SetValue(destination, sourceProp.GetValue(source));                            // :459
        }

Three separate costs per call: two uncached GetProperties() (each returns a fresh array), a LINQ
closure allocation per destination property, and GetValue/SetValue which box every value type.

The other overload, MapInternal<TDest> at src/Mapsicle.Fluent/FluentMapper.cs:468, already does
the right thing: it delegates the bulk copy to the core's compiled source.Map(result) at line 529
and only applies fluent overrides on top. This overload was never given the same treatment.

What should change. Route the conventional property copy through the core's cached, compiled
Map, and keep the fluent-specific work (ignores, conditions, custom mappings, before/after hooks)
as an override pass on top. The structure at lines 522 to 540 is the pattern to follow. Roughly:

typeMap?.GetBeforeMap()?.Invoke(source, destination);

source!.Map(destination);   // core, compiled and cached

if (typeMap != null && HasCustomMappingOrConditions(typeMap, typeof(TDest)))
{
    var applyOverrides = GetOrBuildOverrideAction<TDest>(typeof(TSource), typeMap);
    applyOverrides(source!, destination);
}

typeMap?.GetAfterMap()?.Invoke(source, destination);

Two behaviour differences to watch, because they are the reason this is not a pure refactor:

  • The core Map matches on the source's runtime type, this overload matches on typeof(TSource).
    For a subclass instance passed as its base type the set of copied properties changes.
  • The core Map copies [IgnoreMap]-marked and [MapFrom]-marked properties according to the
    attributes, which this overload ignores entirely today. That is arguably a second bug, and making
    attributes work here is an improvement, but it must be in the CHANGELOG.

How you know you succeeded. Two tests, one for behaviour and one for the property that made this
worth fixing:

[Fact]
public void FluentMap_IntoExistingDestination_AppliesIgnoreAndCustomMappings()
{
    // existing behaviour must not regress: ignores, conditions, ForMember, BeforeMap/AfterMap
}

[Fact]
public void FluentMap_IntoExistingDestination_DoesNotAllocatePerCall()
{
    var mapper = new MapperConfiguration(c => c.CreateMap<User, UserDto>()).CreateMapper();
    var source = new User { /* ... */ };
    var destination = new UserDto();

    mapper.Map(source, destination);   // warm

    GC.Collect(); GC.WaitForPendingFinalizers(); GC.Collect();
    var before = GC.GetAllocatedBytesForCurrentThread();
    for (var i = 0; i < 10_000; i++) mapper.Map(source, destination);
    var bytes = (GC.GetAllocatedBytesForCurrentThread() - before) / 10_000;

    Assert.True(bytes <= 32, $"allocated {bytes} B/call");
}

The allocation test fails today at 776 B/call. It belongs in tests/Mapsicle.Performance.Tests once
issue 10 lands; until then tests/Mapsicle.Fluent.Tests is fine.

Non-obvious thing. tests/Mapsicle.Fluent.Tests/BugfixRegressionTests.cs covers the hook
behaviour that 1.2.3 fixed for this exact overload ("the in-place Map(source, destination) overload
now invokes BeforeMap/AfterMap hooks"). Read it before you restructure, and make sure it still
passes; the hook ordering relative to the property copy is load-bearing, not incidental.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingenhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions