2016-01-01 1 views
1

Я начал работать недавно в новом проекте, где у нас есть тысячи строк устаревшего кода. Мы сталкиваемся с несколькими проблемами производительности. Я решил взглянуть на код и увидел следующее. Существует класс:Возможные проблемы с производительностью с кодом C# в устаревшем проекте

public class BaseDataAccess 
{ 
    private Database dB; 

    public Database DB 
    { 
     get 
     { 
      if (dB == null) 
      { 
       dB = DatabaseFactory.CreateDatabase(); 
      } 
      return dB; 
     } 
    } 
} 

И многие классы потомков, которые наследуются от предыдущего базового класса. Внутри эти другие классы использовать свойства БД, например:

DataSet ds = DB.ExecuteDataSet(spGetCustomersSortedByAge); 

Наконец, существует огромный класс (5000 строк кода) с десятками способов, как следующее:

public void ProcessPayments() 
    { 
     try 
     { 
      List<Employee> employees = new EmployeesDA().SelectAll(null); 
      foreach (Employee employee in employees) 
      { 

       employee.Account = new MovementsDA().SelectAll(employee.Id, DateTime.Now); 

       ... 

       City city = new CitiesDA().Select(zone.cityId); 

       ... 

       Management m = new ManagmentDA().Select(city.id); 

      } 
     } 
     catch (Exception ex) 
     { 
       ... 
     } 

    } 

Примечание в предыдущем методе EmployeesDA, MovementsDA, CitiesDA и ManagmentDA все являются наследниками BaseDataAccess и внутренне используют свои соответствующие свойства БД. Также обратите внимание, что они постоянно создаются внутри циклов foreach (много раз в пределах 2 уровней гнездования).

Я думаю, что сама инстанция подозрительна, но меня больше беспокоит, что происходит с подключениями к базе данных здесь? Является ли каждый экземпляр DA открытием нового базового соединения? Насколько плохо этот код?

В качестве побочного примечания о решении, которое я рассматривал в случае, если этот код должен быть исправлен: я рассматривал возможность создания каждого конструктора частным образом, поэтому компилятор начинает жаловаться на экземпляры и реорганизовывать экземпляры с вызовами метода GetInstance (singleton pattern), чтобы избежать воссоздания объектов и базовых соединений. Но я не уверен, что это может быть опасно, например, если соединения могут быть закрыты. В текущем коде нет этой проблемы из-за того, что все время происходит.

+0

Существует много неправильного подхода к этому подходу, но вы правы, если метод 'DatabaseFactory.CreateDatabase();' возвращает новое подключенное соединение с базой данных для каждого вызова, тогда у вас есть большая проблема с соединением. –

+0

Еще одна проблема, с которой вы столкнулись, - это поддержка транзакций. Не следует ли «ProcessPayments» убедиться, что все происходит в одной транзакции? Или, по крайней мере, одна транзакция на одного сотрудника или какая-то такая вещь? –

+0

Правильный способ справиться с этим заключается в том, чтобы приступить к тому, что у вас есть подключенное состояние, видимое в вашей программе, таким образом, что оно должно управляться. –

ответ

0

Это распространенное заблуждение, что строительство объекта дорого. Это намного дороже, чем базовая арифметика или другие вещи на аппаратном уровне, но вряд ли это прямой источник проблем с производительностью.

Использование целочисленных целых чисел для цикла, например, является расточительным, но построение объекта Employee в каждом vs при повторном использовании изменяемого объекта Employee не даст значимых преимуществ производительности.

Многие сборщики мусора способны к повторному использованию фреймов объектов в таких циклах. По сути, один объектный кадр выделяется и перезаписывается на каждом проходе цикла.

В этом конкретном случае может возникнуть стоимость, если у DA есть значительные затраты на инициализацию. Если это так, я бы реорганизовал код для создания тех, которые были вне цикла. Я бы не использовал фактические статические синглтоны. Я бы использовал инъекцию зависимостей, чтобы управлять объектами singleton, если вам это нужно. Статические синглтоны являются фактически глобальными переменными и являются приглашением к сочетанию состояний и разрыву модульности.

+1

Спасибо, да, я знаю, что Синглтон не очень хорош. Но для этого случая я искал быстрый рефакторинг для проверки производительности и т. Д. Однако мне все же хотелось бы знать, насколько это плохо, когда каждый DA использует класс DatabaseFactory из платформы .NET, чтобы получить соединение все время (сотни экземпляров все вокруг). –

+0

@ StephenH.Anderson профиль и посмотреть, сколько времени потребляют конструкторы. Вам не нужно менять систему, чтобы ее измерить. Вы должны иметь возможность получать минуты/максимальное/среднее время вызова и общее время, затрачиваемое на определенные стеки вызовов. Я подозреваю, что объединение пулов (это первое, что я сделал бы, если это еще не сделано) приведет к значительной амортизации стоимости этих конструкторов. –

Смежные вопросы